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

Repace multiple slash in URI #1111

Merged
merged 1 commit into from
Oct 29, 2018
Merged

Repace multiple slash in URI #1111

merged 1 commit into from
Oct 29, 2018

Conversation

phil-davis
Copy link
Contributor

If the URI contains multiple slashes, then "be nice to the caller" and replace them with a single slash.
e.g. someone that asks for some URI like:

/////abc/def//ghi/jkl///mno//

any extra slashes with empty between them actually make no difference, and we can "compress" them.

Should help in cases where a client is "accidentally" sending bonus slashes or...

e.g. owncloud/core#33119 (comment)

@codecov
Copy link

codecov bot commented Oct 28, 2018

Codecov Report

Merging #1111 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1111   +/-   ##
=========================================
  Coverage     97.19%   97.19%           
  Complexity     2865     2865           
=========================================
  Files           174      174           
  Lines          8057     8057           
=========================================
  Hits           7831     7831           
  Misses          226      226
Impacted Files Coverage Δ Complexity Δ
lib/DAV/Server.php 97.08% <100%> (ø) 191 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa75bde...8728107. Read the comment docs.

Copy link
Member

@staabm staabm left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Lgtm

@DeepDiver1975 DeepDiver1975 merged commit 09ed9f1 into sabre-io:master Oct 29, 2018
@phil-davis phil-davis deleted the replace-multiple-slash-in-uri branch October 29, 2018 10:02
@DeepDiver1975
Copy link
Member

shall we backport this to older releases as well? @staabm

@phil-davis
Copy link
Contributor Author

Backport to 3.2 is in PR #1112
Let me know if older backports are needed.

@staabm
Copy link
Member

staabm commented Oct 29, 2018

shall we backport this to older releases as well?

@DeepDiver1975 good idea. go ahead.

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

3 participants