Fix. Make sure that scheme part of the URI is treated in a case-insensit... #2505
Conversation
|
Fixes #2500. |
|
Paging @sersut @stevendanna @lamont-granquist . |
| # As per: https://github.com/opscode/chef/issues/2500 | ||
| it 'should treat scheme part of the URI in a case-insensitive manner' do | ||
| http = Chef::HTTP.allocate # Calling Chef::HTTP::new sets @url, don't want that. | ||
| expect do |
There was a problem hiding this comment.
I can split this nested expect into two distinct cases, if needed.
|
Note: |
|
This looks good to me - http://tools.ietf.org/html/rfc3986#section-3.1 states that the scheme should be case insensitive, and we should support that. What say you, @jaymzh? Majority maintainer approval needed! 👍 from me. |
kwilczynski
force-pushed
the
kwilczynski/http-create-url
branch
from
0fdbd3e
to
9d4626f
Compare
…sitive manner. This is as per http://en.wikipedia.org/wiki/URI_scheme, and solves some edges i.e., following (30x) URL from the "Location" header where we have to deal with "HTTP://". Signed-off-by: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>
kwilczynski
force-pushed
the
kwilczynski/http-create-url
branch
from
9d4626f
to
31576ab
Compare
|
To my way of thinking I'd say that we should support case insensitive schemes in both cases per the RFC - if there are stylistic reasons for not doing this I'll defer to @sersut et. al. |
|
fix looks obviously correct and has tests... 👍 FWIW, my guess is there's a bunch of similar mistakes scattered around the codebase. |
|
@jonlives @lamont-granquist let me fix |
|
Always side with the RFC. :) 👍 for this. |
|
So @jonlives @lamont-granquist @jaymzh , there is nothing else to fix (i.e., calls to If we have not back port to Chef 11 (client), the simpler way to go about would be to add small monkey-patch fixing Other than that, this is good to go, I believe. |
|
wouldn't worry about Chef-11 backport, this is not a critical bugfix |
|
@tyler-ball you ok with this? If so, will merge it. |
|
|
|
@jonlives you know about updating the md files? |
|
@lamont-granquist que? |
|
The process for Merges... more than just clicking the button... gotta add the CHANGELOG.md and possibly RELEASE_NOTES.md and DOC_UPDATES.md files. and i also rebase all the commits -- that keeps the history clean and also gets it branched off current master and then the travis tests will run again and you can see if someone broke the PR via commits to master (spec test conflicts rather than git merge conflicts). |
|
Gotcha - will get to that later this week most likely. |
|
so usually what i do is create a branch like lcg/2505 to the pr/2505 head and rebase there against origin/master and then add md files and push the new branch. https://help.github.com/articles/checking-out-pull-requests-locally/ really helps. |
Merge pull request #2505 from kwilczynski/http-create-url
|
Merged via #2708 |
...ive manner.
This is as per http://en.wikipedia.org/wiki/URI_scheme, and solves some
edges i.e., following (30x) URL from the "Location" header where we
have to deal with "HTTP://".
Signed-off-by: Krzysztof Wilczynski krzysztof.wilczynski@linux.com