Skip to content

Profiler: Add slash if there is none between host and path #422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 17, 2022

Conversation

ostrolucky
Copy link
Collaborator

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets
Documentation
License MIT

What's in this PR?

See
image
image

Looks like in case target doesn't have leading slash, profiler shows strange URL

@ostrolucky ostrolucky force-pushed the fix-missing-slash-between-host-and-uri branch 2 times, most recently from 51d9cb2 to 2dd5bea Compare September 16, 2022 12:20
@dbu
Copy link
Collaborator

dbu commented Sep 16, 2022

that is confusing. isn't that a bug in the getRequestTarget implementation, or the underlying Uri implementation? looking at https://github.com/Nyholm/psr7/blob/f734364e38a876a23be4d906a2a089e1315be18a/src/RequestTrait.php#L27 , i don't see why the leading / would get lost.

i am a bit reluctant to merge this, as i don't understand how it can happen without a (more severe) bug in the underlying request / uri implemenation. (or if it gets lost in the collector / stack, we should fix it there, but i had a look at those classes and don't see them changing the request target, they seem just data container).

also, the trim would mis-edit some weird urls like //path with double slash: https://3v4l.org/BR9Lp

@ostrolucky
Copy link
Collaborator Author

It's when path of URI is without leading slash. RequestTrait snippet you posted adds leading slash only when path is empty. And we use Guzzle btw, that also behaves this way.

@ostrolucky ostrolucky force-pushed the fix-missing-slash-between-host-and-uri branch from 2dd5bea to 5ed4b8a Compare September 16, 2022 17:14
@ostrolucky
Copy link
Collaborator Author

also, the trim would mis-edit some weird urls like //path with double slash: 3v4l.org/BR9Lp

I've adjusted code for this now

@dbu
Copy link
Collaborator

dbu commented Sep 17, 2022

but how can the path be without leading slash?

ah, or does that happen when the uri is created withPath('no-leading/slash'), for example when the domain is configured as base on the client?

@dbu dbu merged commit d11c111 into master Sep 17, 2022
@dbu dbu deleted the fix-missing-slash-between-host-and-uri branch September 17, 2022 06:53
@ostrolucky
Copy link
Collaborator Author

correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants