Skip to content

Fix issue when http headers are already present#843

Merged
DannyvdSluijs merged 2 commits intojsonrainbow:masterfrom
Seldaek:fix-headers
Sep 9, 2025
Merged

Fix issue when http headers are already present#843
DannyvdSluijs merged 2 commits intojsonrainbow:masterfrom
Seldaek:fix-headers

Conversation

@Seldaek
Copy link
Copy Markdown
Contributor

@Seldaek Seldaek commented Sep 5, 2025

This fixes a regression introduced in #841

If a process/request runs:

  • file_get_contents on an HTTP URL.
  • then a json-schema validation using a file://... URL as schema (not all schema are remotely loaded)
  • and http_get_last_response_headers() is available

Then you end up with:

  • file_get_contents on the URL fills up the headers of that request
  • json-schema validation loads the schema file but using file:// there are no headers, and it seems like file_get_contents does not clear previous headers itself
  • http_get_last_response_headers() then returns the headers of the previous request
  • and then $this->fetchContentType() will fail unless the response headers were of the correct application/schema+json mime type (which is very unlikely).

So this PR clears the headers before we fetch the schema, to make sure we have a clean slate. I'll probably report this to PHP as well because IMO it is a bit surprising.

@DannyvdSluijs
Copy link
Copy Markdown
Collaborator

I’noticed the same also. I’ll have a look this weekend or next week. Would that be ok, or do you need this fixed sooner?

If you could add an entry in the changelog that would help in getting it merged faster.

@Seldaek
Copy link
Copy Markdown
Contributor Author

Seldaek commented Sep 5, 2025

No rush at all, I've downgraded already :)

@DannyvdSluijs DannyvdSluijs merged commit def86e5 into jsonrainbow:master Sep 9, 2025
17 checks passed
@Seldaek
Copy link
Copy Markdown
Contributor Author

Seldaek commented Sep 9, 2025

Thanks for wrapping it up for me

Copilot AI added a commit that referenced this pull request Feb 27, 2026
Co-authored-by: DannyvdSluijs <618940+DannyvdSluijs@users.noreply.github.com>
DannyvdSluijs added a commit that referenced this pull request Feb 27, 2026
…ch (#893)

## Description
See #843 

## Related Issue
Port of #843 
Fixes #860 
Replaces #888 

## Type of Change

<!-- Mark the appropriate option with an "x" -->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Documentation update
- [ ] Code refactoring
- [ ] Other (please describe):

## Checklist

<!-- Mark completed items with an "x" -->

- [x] I have read the [CONTRIBUTING](CONTRIBUTING.md) guidelines
- [x] My code follows the code style of this project
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] All new and existing tests pass
- [x] I have updated the documentation accordingly
- [x] My changes generate no new warnings

## Additional Notes

<!-- Add any additional information that might be helpful for reviewers
-->
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