preserve BC after fix for #74216

[remicollet]

No description provided.

[remicollet]

See

• https://bugs.php.net/74216
• Test failure with 7.1.4RC1 guzzle/guzzle#1790
• Fix #1790 proxy should not end with / guzzle/guzzle#1792

[remicollet]

@krakjoe open as PR as I'd like to get @sgolemon feeling.

If accepted, will have to be cherry-picked in 7.1.4 before release.

[remicollet]

I just change from > 1 to > 2 to avoid using empty string (BTW proxy="/" is obviously wrong)

[krakjoe]

ack, awaiting Sara

[remicollet]

As http://git.php.net/?p=php-src.git;a=commitdiff;h=bab0b99f376dac9170ac81382a5ed526938d595a was applied in 7.0, also needed in 7.0.18 /cc @weltling

[weltling]

@remicollet yeah, it is in there, thanks for the ping.

IMO we should not touch this. We'd otherwise would have to fix any special cases where any other garbage is appended to the port number, etc. AFM, the BC impact is acceptable in this case, as there's a low security impact and the test data is broken itself.

Otherwise, probably a fix to this should be generalized, not checking for an explicit char like '/' at the end, but cutting off the non numeric garbage. Frankly, i'd not see it worth the effort as it might get complicated.

Thanks.

[sgolemon]

Ergh.... um.... I don't think we should preserve BC for transport strings that are broken by design. I regard this as a bug in Guzzle's test. The fact that garbage used to be ignored isn't an endorsement for allowing garbage indefinitely.

[sgolemon]

At the absolute very least, garbage should be cause for raising a warning, so if we do need to pull something like this onto 7.0 and 7.1*, then it should make it clear that those transport strings are malformed.

• The BC argument goes away for 7.2 IMO, since 7.2.0 isn't out yet.

[remicollet]

I think we have consensus on keeping as it is.