Skip to content
This repository has been archived by the owner on Aug 14, 2021. It is now read-only.

dirname() breaks urls on windows #89

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

Conversation

karpour
Copy link

@karpour karpour commented Jul 18, 2019

dirname() seems to be specifically meant for local filesystem operations. On windows, it adds a '\' at the beginning of the returned string, breaking URLs, since the resulted absolute URL looks like http://site.tld/\dirname/ then.

'/' . basename(...) seems to work as a replacement that also works on Windows

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.405% when pulling a0f0bb4 on karpour:master into 23f2175 on andreskrey:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.405% when pulling a0f0bb4 on karpour:master into 23f2175 on andreskrey:master.

@andreskrey
Copy link
Owner

Seems that your change is breaking the tests. Can you take a look at it?

The main problem I see is that the url your change generates has the file path (index.html for example) when it should end at the folder level.

@karpour
Copy link
Author

karpour commented Jul 18, 2019

Yeah, seems, basename isn't the way to go. I can't run the tests atm since in a Windows machine, they won't pass in the first place.
All these functions are meant for local files, not URLs. I'll have a look at it again and see if there's any built-in function that handles everything with forward slashes.

Could simply replace the first character if dirname() with '/', but dirname() doesn't actually always return a string starting with '/', so counting on that would be a mistake as well and it would be a dirty solution.

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

Successfully merging this pull request may close these issues.

3 participants