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

Fix cgi environment url parsing #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adube
Copy link
Contributor

@adube adube commented Jul 5, 2013

We had an issue when trying to use SLD with MapServer via node-mapserv. The SLD_BODY parameter value, which is an encoded SLD XML string, seems to be decoded before being sent to MapServer. A friend of mine, who is unfamiliar with Github, proposed the following fix, which solves the issue.

What do you think ?

@homme
Copy link
Member

homme commented Jul 5, 2013

@adube Thanks, that looks fine except I'd like to de-duplicate the decodeURIComponent(urlParts.pathname) call - I'll add a note in the code. Also could you provide a minimal example of the previously failing SLD URL: I'll add a test that checks for this.

@adube
Copy link
Contributor Author

adube commented Jul 5, 2013

@homme Gotcha. I don't currently have a public data set that I could use to show the the issue, but I'll make sure to build one. I should be able to do that today.

Thanks a lot for the reply! The deployment on a Windows environment is still looking good so far. Terrific !

@homme
Copy link
Member

homme commented Jul 5, 2013

@adube - Thanks, the test would be great. Also I'm glad it's proving useful on Windows. I've made a few changes recently including improving the error reporting and triggering module cleanup on various signals so I hope these have integrated ok. What we need is Travis CI support for Windows :)

@adube
Copy link
Contributor Author

adube commented Jul 5, 2013

I began working with Travis CI a few weeks ago. Do you mean it currently doesn't support Windows or you haven't implemented node-mapserv travis configuration to support Windows in its tests ?

@homme
Copy link
Member

homme commented Jul 5, 2013

I mean unfortunately it doesn't currently support Windows: travis-ci/travis-ci#216

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.

2 participants