-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable encoding on url segments #85
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #85 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 2 2
Lines 324 324
=====================================
Hits 324 324
Continue to review full report at Codecov.
|
primary use case: * regex pattern matches a subset of the URL pathname example (configs): - source: '/foo/:bar*' - destination: '/:bar' - purpose: * 'rewrites' rule would be used to silently ignore the leading 'foo/' directory in path * 'redirects' rule would be used to trigger a 301 redirect to a new URL that removes the leading 'foo/' directory in path example (behavior): - request URL path: '/foo/a/b/c/d/e/f.txt' - :bar interpolates to value (before encoding): 'a/b/c/d/e/f.txt' - :bar interpolates to value (after default encoding): encodeURIComponent('a/b/c/d/e/f.txt') === 'a%2Fb%2Fc%2Fd%2Fe%2Ff.txt' * if the corresponding 'rewrites' or 'redirects' rule includes the flag: {"raw": true} then the raw value will be returned without any encoding based on: * upstream PR 85 vercel/serve-handler#85 references: * pathToRegExp.compile(data, options) https://github.com/pillarjs/path-to-regexp#compile-reverse-path-to-regexp road blocks: * pathToRegExp bug pillarjs/path-to-regexp#260 pillarjs/path-to-regexp#261 status: - the desired behavior will remain broken until this PR is merged - 'source' patterns that match one or more '/' characters cause the library to throw an Error for a failed assertion
I wonder what happened to this PR. Seems still valid to me. |
The downside of this PR is that it doesn't work with glob patterns. For instance:
Will throw this error:
In the meantime, I will use it as is with a regex, as in the test. Here is the patch-package I made for this. You just need to add this file diff --git a/node_modules/serve-handler/src/index.js b/node_modules/serve-handler/src/index.js
index 05e3430..c29cb86 100644
--- a/node_modules/serve-handler/src/index.js
+++ b/node_modules/serve-handler/src/index.js
@@ -66,7 +66,7 @@ const sourceMatches = (source, requestPath, allowSegments) => {
return null;
};
-const toTarget = (source, destination, previousPath) => {
+const toTarget = (source, destination, previousPath, raw) => {
const matches = sourceMatches(source, previousPath, true);
if (!matches) {
@@ -85,7 +85,7 @@ const toTarget = (source, destination, previousPath) => {
props[name] = results[index + 1];
}
- return toPath(props);
+ return toPath(props, raw ? {encode: value => value} : {});
};
const applyRewrites = (requestPath, rewrites = [], repetitive) => {
@@ -101,8 +101,8 @@ const applyRewrites = (requestPath, rewrites = [], repetitive) => {
}
for (let index = 0; index < rewritesCopy.length; index++) {
- const {source, destination} = rewrites[index];
- const target = toTarget(source, destination, requestPath);
+ const {source, destination, raw} = rewrites[index];
+ const target = toTarget(source, destination, requestPath, raw);
if (target) {
// Remove rules that were already applied |
I was trying to rewrite all urls that looked like a/b/:path+ to c/d/:path, so in practice:
a/b/file
=>c/d/file
a/b/dir/dir/dir/file
=>c/d/dir/dir/file
However, it didn't work. After some digging I found that path-to-regexp was encoding the slashes in the matched url segment.
However, you can pass a custom encoder, so I've added an option on rewrite objects to use an identity function (
value => value
), so no character gets encoded in the URL. I've named itraw
but I'm open to suggestions.There's a minor detail, you also have to provide the regex for the destintation, because by default they disallow certain characters there as well. I've added an example to the README mentioning this too. Also, a small test case.