Skip to content
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

Handling urls followed by query params #16

Closed
wants to merge 1 commit into from

Conversation

hulufei
Copy link

@hulufei hulufei commented Mar 8, 2013

Here is the case:

Mapped the remote url to a local directory

Matched url like this: www.address.com/static.css?1.0

Should response static.css in the local directory.

@goddyZhao
Copy link
Owner

Hi, @hulufei, thanks for your pr.

I just wanna catch your mind about the case, do you mean:

  1. you wanna use NProxy to map directory
  2. if the request whose url is in the remote directory and has query string, you still wanna replace it with local one in your directory set via the responder in the NProxy's rule file

Am I right?

@@ -28,7 +28,7 @@ function respond(responderListFilePath){
});

return function respond(req, res, next){
var url = utils.processUrl(req);
var url = utils.processUrl(req).replace(/\?.+/, ''); // remove query params
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The replace pattern is not strict enough, which doesn't cover such url:

http://www.aaa.com/static.css?

@goddyZhao
Copy link
Owner

If my understood is correct, please see my fix fc8639d on it(same solution as yours but with more strict replace pattern ). You can directly update NProxy to apply this fix via npm -g update nproxy

While, if not, please reopen it with more detail information.

Thanks for your pull request again and feel free to contact me with any issues about NProxy.

@goddyZhao goddyZhao closed this Mar 8, 2013
@hulufei
Copy link
Author

hulufei commented Mar 9, 2013

That's exactly what i want, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants