Skip to content

Forward linkType and context query params to resolver URL in getAllHeaders#25

Closed
rmedic wants to merge 2 commits into
gs1:masterfrom
rmedic:fix/php-linktype-context-forwarding
Closed

Forward linkType and context query params to resolver URL in getAllHeaders#25
rmedic wants to merge 2 commits into
gs1:masterfrom
rmedic:fix/php-linktype-context-forwarding

Conversation

@rmedic

@rmedic rmedic commented Apr 14, 2026

Copy link
Copy Markdown

The JS test runner sends linkType, context, mediaType and lang as separate query parameters to tester.php (e.g. ?test=getAllHeaders&testVal=... &linkType=gs1:pip&lang=de). Previously, linkType and context were parsed but silently discarded, so the cURL request to the resolver was made against the bare URL without ?linkType=... This caused the resolver to redirect to its default link rather than the requested link type, producing false failures for all per-link-type redirect tests.

Fixed by appending linkType and context as query parameters to the resolver URI before the cURL call. mediaType and lang correctly remain as Accept and Accept-Language HTTP headers.

…aders

The JS test runner sends linkType, context, mediaType and lang as separate
query parameters to tester.php (e.g. ?test=getAllHeaders&testVal=...
&linkType=gs1:pip&lang=de). Previously, linkType and context were parsed
but silently discarded, so the cURL request to the resolver was made
against the bare URL without ?linkType=... This caused the resolver to
redirect to its default link rather than the requested link type, producing
false failures for all per-link-type redirect tests.

Fixed by appending linkType and context as query parameters to the resolver
URI before the cURL call. mediaType and lang correctly remain as Accept and
Accept-Language HTTP headers.
@sboeckelmann

Copy link
Copy Markdown
Contributor

+1 - please accept and deploy

@nicklansley nicklansley self-assigned this May 19, 2026
@nicklansley nicklansley requested a review from Copilot May 19, 2026 14:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR ensures linkType and context query parameters provided to tester.php for the getAllHeaders test are forwarded to the resolver URL used by getCustomHeaders, preventing incorrect redirects and false failures in per-link-type redirect tests.

Changes:

  • Build a resolver URI by appending linkType and context as query parameters before issuing the cURL request.
  • Keep mediaType and lang mapped to Accept and Accept-Language request headers (now passed as optional values).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tester.php
Comment thread tester.php Outdated
…d query strings

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@nicklansley

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

@louisaxel-ambroise

Copy link
Copy Markdown
Contributor

I overlooked this merge request while resolving some issues on my side, and I think that #30 resolved the same issue as that one

It looks like it's not necessary to update the php file for this. The problem seems to be just a mismatch on the query string parameters of the digital link URL that needs to be url-encoded and on the request sent to the php url the are not encoded.

Happy to cancel my merge request if needed, but for me these changes being more complexity than needed.

Thanks!

@philarcher

Copy link
Copy Markdown
Member

Thanker very much @rmedic. Having now accepted all the other recent PRs, can you please confirm that this PR is no longer required?

@rmedic rmedic closed this Jun 15, 2026
@rmedic rmedic deleted the fix/php-linktype-context-forwarding branch June 15, 2026 10:48
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.

6 participants