yii2
yii2 copied to clipboard
yiisoft
#20199 Reverted BaseUrl::isRelative() behaviour to previous 2.0.49 ve…
…rsion due to yii\web\UrlManager::createAbsoluteUrl() malfunction depending on this.
| Q | A |
|---|---|
| Is bugfix? | ✔️ |
| New feature? | ❌ |
| Breaks BC? | ❌ |
| Fixed issues | #20199 |
Ok @samdark, I will at a certain point today. I've just given a brief look at them and seems a couple of tests are starting from the wrong assumption links like acme.com/test?tnt-link=https://tnt.com/ are valid URIs; accordingly to rfc3986 (T. Berners-Lee) they are not: their 'legal' counterpart would be instead the Percent-Encoding version of it: acme.com/test?tnt-link=https%3A%2F%2Ftnt.com%2F
But I'll let you know as soon as I can give a deeper look into it.
I'm not sure why do you think that acme.com/test?tnt-link=https://tnt.com/ is not valid. This RFC explicitly mentions that you can skip encoding for readability:
https://stackoverflow.com/a/68151221/5812455
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 64.99%. Comparing base (
ec46ede) to head (9f74eba). Report is 24 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #20203 +/- ##
============================================
- Coverage 64.99% 64.99% -0.01%
- Complexity 11391 11393 +2
============================================
Files 430 430
Lines 36923 36925 +2
============================================
Hits 23998 23998
- Misses 12925 12927 +2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
As promised, on Friday I've dug deeper and found out the problem was in fact the a non rfc3986 compliant url in the tests. I've changed that one, and for sake of clarity I've also added an additional case aimed to test a more relaxed version of rfc3986, but still legal. Everything's is committed now in the pr.
@edegaudenzi You may argue that acme.com/test?tnt-link=https://tnt.com/ is not a valid URL, but it is correctly handled by many tools (PHP included) and it is definitely not an absolute URL. Your PR introduces regression that treats acme.com/test?tnt-link=https://tnt.com/ as absolute URL (and thus breaking ensureScheme()) and you altered tests to hide this regression.