Skip to content

fix: stop encoding '.' to '%2E' #1766

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

Merged
merged 1 commit into from
Dec 21, 2021
Merged

fix: stop encoding '.' to '%2E' #1766

merged 1 commit into from
Dec 21, 2021

Conversation

JohnVillalovos
Copy link
Member

@JohnVillalovos JohnVillalovos commented Dec 19, 2021

Forcing the encoding of '.' to '%2E' causes issues. It also goes
against the RFC:
https://datatracker.ietf.org/doc/html/rfc3986.html#section-2.3

From the RFC:
For consistency, percent-encoded octets in the ranges of ALPHA
(%41-%5A and %61-%7A), DIGIT (%30-%39), hyphen (%2D), period (%2E),
underscore (%5F), or tilde (%7E) should not be created by URI
producers...

Closes #1006
Related #1356
Related #1561

BREAKING CHANGE: stop encoding '.' to '%2E'. This could potentially be
a breaking change for users who have incorrectly configured GitLab
servers which don't handle period '.' characters correctly.

@JohnVillalovos JohnVillalovos marked this pull request as draft December 19, 2021 23:23
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/leave_dot branch 2 times, most recently from 950b7ba to d12b829 Compare December 19, 2021 23:38
@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2021

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.95%. Comparing base (ccefe80) to head (702e41d).
Report is 1215 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1766      +/-   ##
==========================================
- Coverage   91.97%   91.95%   -0.02%     
==========================================
  Files          76       76              
  Lines        4759     4751       -8     
==========================================
- Hits         4377     4369       -8     
  Misses        382      382              
Flag Coverage Δ
cli_func_v4 81.30% <100.00%> (-0.04%) ⬇️
py_func_v4 80.67% <100.00%> (-0.04%) ⬇️
unit 83.20% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
gitlab/client.py 90.00% <100.00%> (-0.11%) ⬇️
gitlab/utils.py 88.00% <100.00%> (-1.66%) ⬇️

@JohnVillalovos JohnVillalovos changed the title Jlvillal/leave dot chore: stop encoding '.' to '%2E' Dec 19, 2021
@JohnVillalovos JohnVillalovos marked this pull request as ready for review December 19, 2021 23:52
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/leave_dot branch 2 times, most recently from d7893c1 to 39155df Compare December 20, 2021 00:15
@nejch
Copy link
Member

nejch commented Dec 20, 2021

@JohnVillalovos could you make this a fix type so it shows in the release and also add the stop encoding '.' to '%2E' sentence at the beginning of the breaking change trailer so it's clear in the release notes in the breaking changes list?

@JohnVillalovos
Copy link
Member Author

@JohnVillalovos could you make this a fix type so it shows in the release and also add the stop encoding '.' to '%2E' sentence at the beginning of the breaking change trailer so it's clear in the release notes in the breaking changes list?

Done I think. Thanks.

Forcing the encoding of '.' to '%2E' causes issues. It also goes
against the RFC:
https://datatracker.ietf.org/doc/html/rfc3986.html#section-2.3

From the RFC:
  For consistency, percent-encoded octets in the ranges of ALPHA
  (%41-%5A and %61-%7A), DIGIT (%30-%39), hyphen (%2D), period (%2E),
  underscore (%5F), or tilde (%7E) should not be created by URI
  producers...

Closes #1006
Related #1356
Related #1561

BREAKING CHANGE: stop encoding '.' to '%2E'. This could potentially be
a breaking change for users who have incorrectly configured GitLab
servers which don't handle period '.' characters correctly.
@JohnVillalovos JohnVillalovos changed the title chore: stop encoding '.' to '%2E' fix: stop encoding '.' to '%2E' Dec 21, 2021
@nejch nejch merged commit eef8059 into main Dec 21, 2021
@nejch nejch deleted the jlvillal/leave_dot branch December 21, 2021 13:57
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.

Creation project with "." in name works incorrectly.
3 participants