Added additional test case for UrlGenerator class

[arvislacis]

I tested the latest 32df10c commit and it seems that issues described in wintercms/wn-translate-plugin#36 is not solved.

So I added additional test case.

[arvislacis]

Added test cases fail so issue still exists (or I made super dummy mistake in these tests).

wintercms/wn-translate-plugin#36 issue also mostly focus about this case because although initial filter URL is like https://www.example.com/path?array[]=v1&array[]=v2 - later it gets replaced to https://www.example.com/path?array[0]=v1&array[1]=v2 because of language switch component... https://www.example.com/path?array[]=v1&array[]=v2 and https://www.example.com/path?array[0]=v1&array[1]=v2 is same but Redirect::to() then translates it to https://www.example.com/path?array[0][]=v1&array[1][]=v2 which is not correct.

[mjauvin]

I confirm.

[mjauvin]

@arvislacis please add this to this PR, I think it fixes the issue:

diff --git a/src/Router/UrlGenerator.php b/src/Router/UrlGenerator.php
index 5c6f4a8..7268b46 100644
--- a/src/Router/UrlGenerator.php
+++ b/src/Router/UrlGenerator.php
@@ -326,7 +326,7 @@ public static function buildStr(array $query, string $prefix = '', $argSeparator
             if ($i === $k) {
                 if ($prefix) {
                     // Make sure the key is setup for array values
-                    if (Str::endsWith($prefix, '[]')) {
+                    if (Str::endsWith($prefix, ']')) {
                         $key = $prefix;
                     } else {
                         $key = "{$prefix}[]";

[arvislacis]

@mjauvin Done in 5be073c As far as I tested then this solves my issue (wintercms/wn-translate-plugin#36) but I would suggest someone else also to take look.

Also added another test case (f329c19) for

storm/src/Router/UrlGenerator.php

Line 341 in 32df10c

$key = $prefix ? "{$prefix}[{$k}]" : $k;

^ Everything is fine for this, I think test cases wouldn't be bad.

Probably there could also be tests for

storm/src/Router/UrlGenerator.php

Line 332 in 32df10c

$key = "{$prefix}[]";

and

storm/src/Router/UrlGenerator.php

Line 336 in 32df10c

$key = $v;

although at the moment I couldn't think of any real life examples.

[arvislacis]

@mjauvin A little bit off-topic but is there any particular reason Winter CMS has implemented it's own URL processing and decoding, encoding and not using the Laravel one? I understand that users are nasty and can insert a lot of interesting things in URLs so decoding/encoding and additional processing may avoid this.

For example, if we look at my Locale Switcher case then for some users it also may look strange that [] gets encoded and displayed as %5B0%5D - of course that's nothing too critical but still...

[LukeTowers]

@arvislacis it's because Laravel does not do any of the parsing or url encoding / decoding of URLs that Winter is doing. There is no guarantee in Laravel when you passing a value to Url::to() (or any variation) that you will get a valid, safe URL back.

I personally wanted a helper that would guarantee that the output generated by it was a spec-following URL that could be safely used in element attributes and anywhere else you might need a URL without worrying about potential injection attacks from users controlling part of or the entire URL.

[arvislacis]

@LukeTowers Thanks for clarification.

[LukeTowers]

Thanks for the help @arvislacis & @mjauvin!