fix: [Issue-2437] Rewrites should support special symbols in dynamic routes

[savvaleukhin]

A proposal for fixing «Rewrites don't work with special symbols in a URL»
Issue reference.

Summary of the issue:

1. no-op rewrites don't work with paths that include symbols like @ and =
2. external rewrites will build incorrect destination routes for paths that include symbols like @ and =
   all of the above works in the Next.js server

I followed the code and found no reason for using encoders in the rewrite mechanism.
the next code:

const match = matchPath(path, rewrite.source);
if (!match) {
  continue;
}

should prevent wrong path matching from the beginning.

Also, the compile function should not produce an invalid path since validation is enabled by default

@dphang Am I missing something?

[slsnextbot]

Handler Size Report

No changes to handler sizes.

Base Handler Sizes (kB) (commit e6367b5)

{
    "Lambda": {
        "Default Lambda": {
            "Standard": 1578,
            "Minified": 692
        },
        "Image Lambda": {
            "Standard": 1543,
            "Minified": 831
        }
    },
    "Lambda@Edge": {
        "Default Lambda": {
            "Standard": 1588,
            "Minified": 698
        },
        "Default Lambda V2": {
            "Standard": 1580,
            "Minified": 694
        },
        "API Lambda": {
            "Standard": 634,
            "Minified": 318
        },
        "Image Lambda": {
            "Standard": 1551,
            "Minified": 835
        },
        "Regeneration Lambda": {
            "Standard": 1233,
            "Minified": 566
        },
        "Regeneration Lambda V2": {
            "Standard": 1307,
            "Minified": 596
        }
    }
}

New Handler Sizes (kB) (commit 182ac60)

{
    "Lambda": {
        "Default Lambda": {
            "Standard": 1578,
            "Minified": 692
        },
        "Image Lambda": {
            "Standard": 1543,
            "Minified": 831
        }
    },
    "Lambda@Edge": {
        "Default Lambda": {
            "Standard": 1588,
            "Minified": 698
        },
        "Default Lambda V2": {
            "Standard": 1580,
            "Minified": 694
        },
        "API Lambda": {
            "Standard": 634,
            "Minified": 318
        },
        "Image Lambda": {
            "Standard": 1551,
            "Minified": 835
        },
        "Regeneration Lambda": {
            "Standard": 1233,
            "Minified": 566
        },
        "Regeneration Lambda V2": {
            "Standard": 1307,
            "Minified": 596
        }
    }
}

[codecov]

Codecov Report

Merging #2440 (a455602) into master (e6367b5) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head a455602 differs from pull request most recent head 182ac60. Consider uploading reports for the commit 182ac60 to get more accurate results

@@            Coverage Diff             @@
##           master    #2440      +/-   ##
==========================================
+ Coverage   83.85%   83.88%   +0.02%     
==========================================
  Files         102      102              
  Lines        3717     3717              
  Branches     1191     1191              
==========================================
+ Hits         3117     3118       +1     
+ Misses        588      587       -1     
  Partials       12       12              

Impacted Files	Coverage Δ
packages/libs/core/src/match.ts	100.00% <100.00%> (ø)
packages/libs/core/src/route/rewrite.ts	100.00% <100.00%> (ø)
packages/libs/core/src/images/imageOptimizer.ts	84.51% <0.00%> (+0.41%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6367b5...182ac60. Read the comment docs.

[lucasbaze]

Question: Instead of removing the encoding entirely, would it make sense to use encodeURI?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent

[savvaleukhin]

Hi, thanks for the suggestion.

I've checked how the next.js server works with path matching and the difference is quite significant.
However, the usage of encodeURI makes the behavior a little bit closer.
It will encode not-ASCII chars but will not encode symbols like @=.