Use Guzzle to resolve the URL against the base URL #51799
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello Reviewers 👋
This work extends the support for RFC3986 from Guzzle Client to the Http Client.
Previously @JaZo tried to implement this but it was partially addressed by #41307 in version 9 and not merged for version 10 #41289.
The RFC3986 that Guzzle implements allows us to do something that currently isn't possible because this code:
framework/src/Illuminate/Http/Client/PendingRequest.php
Lines 887 to 891 in 741560f
🔴 First: overriding the base path (initial slash of URI)
this is really helpful when you have special routes (baseUri: http://foo.com/api/v1 but you would like to call http://foo.com/health-check)
🔴 Second: overriding the last part of the path (no trailing slash in baseURI)
💡 Solution
If it is set, pass the
PendingRequest:baseUrl
into the options for the Guzzle client and delegate to it the resolution of the final URI . If thebase_uri
is explicitly set in the$options
for a request, this takes precedence over thePendingRequest:baseUrl
. By doing this, we also have support foridn_conversion
and the addition of the schema if it's missing thanks to Guzzle's Client::buildUrl(...)This is the table taken from Guzzle doc :
🟢 Adopting an RFC and having consistency with the underlying Guzzle client, I think, will help future developers.
For this reason, I have targeted the PR to master and not to 11.x or 10.x, but if it can considered a bugfix I'm happy to cherry-pick this commit.