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.

Sign up for GitHub

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

Jump to bottom

href is invalid when there is query string in url #85

Closed
leesei opened this issue Oct 10, 2013 · 0 comments
Closed

href is invalid when there is query string in url #85

leesei opened this issue Oct 10, 2013 · 0 comments

Comments

@leesei
Copy link
Contributor

leesei commented Oct 10, 2013

Start ecstatic with --showDir, open this url:

http://localhost:8080/?key=value

The href's got ?key=value within, which is invalid.

http://localhost:8080/?key=value/folder%20with%20space/

Changing this in showdir.js:writeRow() will ignore the query string:

-                req.url.replace(/\/$/, '')
+                parsed.pathname.replace(/\/$/, '')

But the test 'malformed showdir uri' will then fail, I think this test is incorrect in putting the malformed string in the query.
The test failed incidentally in the first place because of this issue.

We already protect against invalid pathname in ecstatic.js:middleware(). We could check the query part by passing req.url there.

I would suggest checking req.url in ecstatic.js:middleware(), and test malformed string in both query and pathname in malformed-dir.js.
This was referenced Oct 10, 2013
Closed
Closed
Closed
jfhbrook pushed a commit that referenced this issue Oct 21, 2013
@leesei @jfhbrook
Fixes #84, #85, #86
fc20c90 
url validation should be done in `ecstatic.js` (#85)
Fix redirect location (use `parsed.pathname` instead of urldecoded `pathname`) (#86)

Fix encoding of href genereated in `writeRow()`, no longer need `decodeURI()` (hence error check) here (#86)
Query string is appended to href (if applicable) (#85)
Use html5 doctype and utf-8 encoding (#84)
Use `<table>` to in `render()` (so we can add more columns later on)
Add '/' to dir entries (both link and display name)
Add some comments
Fixed some linter warnings

Fixed test case for 'subdir_with space' (#86)
Added test case for non-ascii url (#86)

 On branch issue84-85-86
 Changes to be committed:
	modified:   lib/ecstatic.js
	modified:   lib/ecstatic/showdir.js
	modified:   test/core.js
	new file:   test/public/subdir_with space/file_with space.html
	new file:   "test/public/\344\270\255\346\226\207/\346\252\224\346\241\210.html"
@leesei leesei closed this as completed Oct 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@leesei