Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

directory middleware generates invalid links with \ on Windows 7 #630

Closed
dmose opened this Issue Jul 12, 2012 · 4 comments

Comments

Projects
None yet
3 participants

dmose commented Jul 12, 2012

Tested on:

Windows 7
connect 2.3.5 and 2.3.8
Node 0.8.2

An example link:

<a href="%5Cexamples" class="" title="examples">examples</a>

where %5C is a the URL-encoding for \

Interestingly, URLs of this sort cause those links to be broken on Firefox, but not on Chrome. I suspect Chrome is being more liberal, and Firefox more strict in standards compliance, but I haven't verified this.

dmose commented Jul 12, 2012

https://github.com/senchalabs/connect/blob/master/lib/middleware/directory.js#L175 is the location of the bug. path.join is being used, and it normalizes to the local machine's filesystem conventions, so any / characters in the dir argument are converted to \

The fix is to replace

join(dir, file)

with

dir + file

dmose commented Jul 13, 2012

@jbuck points out that a fix that would preserve the existing semantics (the fact that join calls normalize to resolve stuff like "..") could be preserved more precisely by leaving the join in place and then using a regex to transliterate \ to /. This doesn't quite feel right to me, but maybe it is. My gut is that something like a normalizeURLPath function would be cleaner. Does one of the maintainers have an opinion on this, or an alternate suggestion?

Member

tj commented Jul 16, 2012

my bad yeah we should just concat them

centi commented Feb 20, 2013

Is there any progress on this bug? I've just experienced the same problem and it makes the directory middleware quite unusable.

The generated URLs (those with backslashes) may work in some browsers, but that is not relevant. Only forward slashes can be used to define hierarchy in URLs.

@jonathanong jonathanong closed this in #914 Oct 15, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment