lexbor: Merge upstream fixes (8.5) - #23660
Conversation
alexandre-daubois
changed the title
d3dac2c to
d18b498
Compare
There was a problem hiding this comment.
FWIW: Historically we didn't add tests for upstream issues (because the fix is already tested upstream), so for all I care these can be dropped.
There was a problem hiding this comment.
FWIW: I'm happy with these tests, because these make sure that no matter which library or library version we use under the hood, the asserted edge cases satisfy the related specifications, and most tests already check lexbor's and uriparser's own behavior, not just the php-src integration.
| --FILE-- | ||
| <?php | ||
|
|
||
| $url = Uri\WhatWg\Url::parse("https://" . str_repeat("é", 5000) . ".com/"); |
There was a problem hiding this comment.
| $url = Uri\WhatWg\Url::parse("https://" . str_repeat("é", 5000) . ".com/"); | |
| $url = new Uri\WhatWg\Url("https://" . str_repeat("é", 5000) . ".com/"); |
If this test is kept, use the throwing variant of the constructor (makes it easier to debug when parsing fails).
There was a problem hiding this comment.
Ah, I see that this is consistent with existing tests. Perhaps we should adjust that for all tests. What do you think @kocsismate?
There was a problem hiding this comment.
Yes, I was also thinking about changing most test to use the constructor, except for a few which make sure that parse() also works as intended ^^
|
@alexandre-daubois FYI: lexbor/lexbor#396 is another candidate which should also land when merged. |
|
Thanks Máté! Keeping an eye on it |
3 participants
Once this one and #23659 are merged, I'll have a look to actually bump lexbor to 3.0.1 on master.