New Symfony HTTP client transport not always reliable

[FelixJacobi]

At least in the integration test a checksum error occurs some times. See https://github.com/littleredbutton/bigbluebutton-api-php/pull/67#issuecomment-847315036 for details,

[FelixJacobi]

Sample extracted with some debug statements inside the CurlHttpClient:

 ✘ Create meeting with document embedded
   ┐
   ├ Failed asserting that two strings are equal.
   ┊ ---·Expected
   ┊ +++·Actual
   ┊ @@ @@
   ┊ -'SUCCESS'
   ┊ +'FAILED'
   │
   ╵ /home/VERTRIEB/felix.jacobi/git/lib-upstream/bigbluebutton-api-php/tests/integration/AbstractBigBlueButtonIntegrationTest.php:154
   ┴
bool(false)
string(1465) "https://bbb0.iserv.eu/bigbluebutton/api/create?name=Dr.+Marcelle+McGlynn+V&meetingID=84be7c90-018d-36e3-9de2-abe4fe03838b&attendeePW=.wL%27Z%25%26%7CCCpmCc%2F7&moderatorPW=y%25%23t%7EGCyl%249LPF.JS&welcome=Veniam+et+autem+tempora+iure+distinctio+quia.&dialNumber=%28259%29+910-4889+x280&voiceBridge=70788&maxParticipants=10&logoutURL=https%3A%2F%2Frenner.com%2Fnobis-quia-pariatur-voluptas.html&record=true&duration=691&moderatorOnlyMessage=Et+rem+quia+nam.&autoStartRecording=false&allowStartStopRecording=true&webcamsOnlyForModerator=true&logo=https%3A%2F%2Florempixel.com%2F330%2F70%2F%3F49071&bannerText=Fuga+autem+dolor+suscipit+et+dolor+officiis.&bannerColor=%2344203d&copyright=Blanditiis+in+ratione+hic+velit+autem+ut+ratione.+Consequatur+eos+dolores+minima+nisi+quibusdam+itaque.+Sit+impedit+aut+odio+at+illum+qui+nihil.&muteOnStart=false&allowModsToUnmuteUsers=true&lockSettingsDisableCam=false&lockSettingsDisableMic=false&lockSettingsDisablePrivateChat=true&lockSettingsDisablePublicChat=true&lockSettingsDisableNote=true&lockSettingsLockedLayout=false&lockSettingsHideUserList=false&lockSettingsLockOnJoin=false&lockSettingsLockOnJoinConfigurable=false&guestPolicy=ALWAYS_ACCEPT&meta_endCallbackUrl=http%3A%2F%2Fokeefe.com%2Feos-consequuntur-autem-alias-eum-consequatur-placeat-quia&meta_bbb-recording-ready-url=https%3A%2F%2Fbeier.net%2Fdolorum-autem-modi-aut.html&meta_presenter=Prof.+Torrey+Bogisich&checksum=b28ea90583f78d999aca2c5d234388bd53318f5b"
string(1463) "https://bbb0.iserv.eu/bigbluebutton/api/create?name=Dr.+Marcelle+McGlynn+V&meetingID=84be7c90-018d-36e3-9de2-abe4fe03838b&attendeePW=.wL%27Z%25%26%7CCCpmCc%2F7&moderatorPW=y%25%23t~GCyl%249LPF.JS&welcome=Veniam+et+autem+tempora+iure+distinctio+quia.&dialNumber=%28259%29+910-4889+x280&voiceBridge=70788&maxParticipants=10&logoutURL=https%3A%2F%2Frenner.com%2Fnobis-quia-pariatur-voluptas.html&record=true&duration=691&moderatorOnlyMessage=Et+rem+quia+nam.&autoStartRecording=false&allowStartStopRecording=true&webcamsOnlyForModerator=true&logo=https%3A%2F%2Florempixel.com%2F330%2F70%2F%3F49071&bannerText=Fuga+autem+dolor+suscipit+et+dolor+officiis.&bannerColor=%2344203d&copyright=Blanditiis+in+ratione+hic+velit+autem+ut+ratione.+Consequatur+eos+dolores+minima+nisi+quibusdam+itaque.+Sit+impedit+aut+odio+at+illum+qui+nihil.&muteOnStart=false&allowModsToUnmuteUsers=true&lockSettingsDisableCam=false&lockSettingsDisableMic=false&lockSettingsDisablePrivateChat=true&lockSettingsDisablePublicChat=true&lockSettingsDisableNote=true&lockSettingsLockedLayout=false&lockSettingsHideUserList=false&lockSettingsLockOnJoin=false&lockSettingsLockOnJoinConfigurable=false&guestPolicy=ALWAYS_ACCEPT&meta_endCallbackUrl=http%3A%2F%2Fokeefe.com%2Feos-consequuntur-autem-alias-eum-consequatur-placeat-quia&meta_bbb-recording-ready-url=https%3A%2F%2Fbeier.net%2Fdolorum-autem-modi-aut.html&meta_presenter=Prof.+Torrey+Bogisich&checksum=b28ea90583f78d999aca2c5d234388bd53318f5b"

first: URL as passed by us. second: URL after Symfony HTTP client. On some point two characters were lost.

[SamuelWei]

Great job tracking the issue

[sualko]

On some point two characters were lost.

I think the url was reencoded. %7E --> ~. Not sure why we encode the tilde in the first place.

[FelixJacobi]

On some point two characters were lost.

I think the url was reencoded. %7E --> ~. Not sure why we encode the tilde in the first place.

I guess it has something to do with how Symfony HTTP client builds query strings:

    /**
     * Merges and encodes a query array with a query string.
     *
     * @throws InvalidArgumentException When an invalid query-string value is passed
     */
    private static function mergeQueryString(?string $queryString, array $queryArray, bool $replace): ?string
    {
        if (!$queryArray) {
            return $queryString;
        }

        $query = [];

        if (null !== $queryString) {
            foreach (explode('&', $queryString) as $v) {
                if ('' !== $v) {
                    $k = urldecode(explode('=', $v, 2)[0]);
                    $query[$k] = (isset($query[$k]) ? $query[$k].'&' : '').$v;
                }
            }
        }

        if ($replace) {
            foreach ($queryArray as $k => $v) {
                if (null === $v) {
                    unset($query[$k]);
                }
            }
        }

        $queryString = http_build_query($queryArray, '', '&', \PHP_QUERY_RFC3986);
        $queryArray = [];

        if ($queryString) {
            foreach (explode('&', $queryString) as $v) {
                $queryArray[rawurldecode(explode('=', $v, 2)[0])] = $v;
            }
        }

        return implode('&', $replace ? array_replace($query, $queryArray) : ($query + $queryArray));
    }

(specially the $encoding_type_argument of http_build_query set to PHP_QUERY_RFC3986).

I guess we can adapt this on our code if this means no harm for BBB to understand the request correctly.

[FelixJacobi]

See HTTP build query with PHP_QUERY_RFC3986 before PHP 5.4 - Stack Overflow for backgrounds. As this changes the behavior for + and ~ it fits to @sualko's comment. I guess it should be fine to use RFC 3986 here. Falling back to the old-style query parameters here I guess is only done as it was the PHP default for a long time.

[SamuelWei]

I see, I locally added ,'', '&', \PHP_QUERY_RFC3986); to the three time we use \http_build_query and the error did not occur again

[FelixJacobi]

See #75.

[SamuelWei]

See #75.

Haha, I just created a branch and saw your comment before I started to implement it