Fix regexp to use case-insensitivity correctly #889

Merged
merged 1 commit into from May 9, 2016

Conversation

Projects
None yet
5 participants
@rennex
Contributor

rennex commented Jun 19, 2014

I noticed a faulty regexp in Sinatra's uri():
The regexp /[A-z]/ matches all alphabets, but also six non-alphabetic characters, as demonstrated by ("A".."z").to_a.join
=> "ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmnopqrstuvwxyz"

Fix regexp to use case-insensitivity correctly
The regexp /[A-z]/ matches all alphabets, but also six non-alphabetic
characters, as demonstrated by ("A".."z").to_a.join
=> "ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz"
@kytrinyx

This comment has been minimized.

Show comment
Hide comment
@kytrinyx

kytrinyx Jun 20, 2014

Member

Good catch, thank you.

There's a failure in the integration test (test/integration_test.rb:92). Would you take a look at that?

Also, we might want a unit test to protect against regressions here. What do you think, @rkh? Too paranoid?

Member

kytrinyx commented Jun 20, 2014

Good catch, thank you.

There's a failure in the integration test (test/integration_test.rb:92). Would you take a look at that?

Also, we might want a unit test to protect against regressions here. What do you think, @rkh? Too paranoid?

@rennex

This comment has been minimized.

Show comment
Hide comment
@rennex

rennex Jun 20, 2014

Contributor

The test failure from Travis seems unrelated to my changes. I copied the relevant part here, these two warnings were the cause:

lib/sinatra/base.rb:1744: warning: loading in progress, circular require considered harmful - /home/travis/.rvm/gems/ruby-1.9.2-p320/gems/rack-1.5.2/lib/rack/logger.rb
lib/sinatra/base.rb:1733: warning: loading in progress, circular require considered harmful - /home/travis/.rvm/gems/ruby-1.9.2-p320/gems/rack-1.5.2/lib/rack/nulllogger.rb

Also, the tests ran successfully on my own machine. No idea what causes those...

Contributor

rennex commented Jun 20, 2014

The test failure from Travis seems unrelated to my changes. I copied the relevant part here, these two warnings were the cause:

lib/sinatra/base.rb:1744: warning: loading in progress, circular require considered harmful - /home/travis/.rvm/gems/ruby-1.9.2-p320/gems/rack-1.5.2/lib/rack/logger.rb
lib/sinatra/base.rb:1733: warning: loading in progress, circular require considered harmful - /home/travis/.rvm/gems/ruby-1.9.2-p320/gems/rack-1.5.2/lib/rack/nulllogger.rb

Also, the tests ran successfully on my own machine. No idea what causes those...

@vipulnsward

This comment has been minimized.

Show comment
Hide comment
@vipulnsward

vipulnsward Jun 20, 2014

Contributor

Warning seems unrelated to this patch.

Contributor

vipulnsward commented Jun 20, 2014

Warning seems unrelated to this patch.

@rkh

This comment has been minimized.

Show comment
Hide comment
@rkh

rkh Jun 20, 2014

Member

I have no idea where this issue comes from all of a sudden.

Test would handy, indeed.

Unless I'm missing something, the main difference will be that "htt^p://foo" is treated as a relative link, correct?

Member

rkh commented Jun 20, 2014

I have no idea where this issue comes from all of a sudden.

Test would handy, indeed.

Unless I'm missing something, the main difference will be that "htt^p://foo" is treated as a relative link, correct?

@zzak zzak added the feedback label Feb 6, 2015

@zzak zzak merged commit c57cad9 into sinatra:master May 9, 2016

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details

zzak added a commit that referenced this pull request May 9, 2016

@zzak zzak added this to the 2.0.0 milestone Aug 21, 2016

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