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

Downcase URLs in normalization #23

Merged
merged 1 commit into from
Jan 13, 2015
Merged

Downcase URLs in normalization #23

merged 1 commit into from
Jan 13, 2015

Conversation

jage
Copy link
Contributor

@jage jage commented Jan 13, 2015

URLs are not case-insensitive, but since our normalized URLs are just
used internally, it's ok. The original URL should always be used when
actually requesting a resource.

Close #8

@jage
Downcase URLs in normalization
403b006 
URLs are not case-insensitive, but since our normalized URLs are just
used internally, it's ok. The original URL should always be used when
actually requesting a resource.

Close #8
@dentarg
Copy link
Collaborator

dentarg commented Jan 13, 2015

Hmm.

@dentarg
Copy link
Collaborator

dentarg commented Jan 13, 2015

Hmm.

I was thinking of the fact that Ruby #downcase doesn't handle UTF-8.

pry(main)> Twingly::URL::Normalizer.normalize("http://www.fooBARfooÅÄÖfoo.com").first.downcase
=> "http://www.foobarfooÅÄÖfoo.com/"

I don't think that's a stopper here, I just wanted to make sure it didn't blow up.

@jage
Copy link
Contributor Author

jage commented Jan 13, 2015

Good thinking.

jage added a commit that referenced this pull request Jan 13, 2015
@jage
Merge pull request #23 from twingly/downcase-url
56c57f1 
Downcase URLs in normalization
@jage jage merged commit 56c57f1 into master Jan 13, 2015
@jage jage deleted the downcase-url branch January 13, 2015 14:09
@walro
Copy link
Contributor

walro commented Sep 9, 2015

Hmm, I wonder what our .NET implementation does here actually. Not entirely sure it down-cases.... We need to take alook at this in #12 too.

I think we downcased here to solve an issue in BRM.

roback pushed a commit that referenced this pull request Sep 9, 2015
@twingly-mob @roback
Do not downcase URL path
1e7c934 
This is the same behavior as the .NET normalization.

Part of #12.

Related to #8, #9 (and #23).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Always return normalized urls in lower case
3 participants
@jage @dentarg @walro