Skip to content

Other content types support - #2

Merged
dacap merged 2 commits into
igarastudio:mainfrom
martincapello:other-content-types-support
Mar 18, 2026
Merged

Other content types support#2
dacap merged 2 commits into
igarastudio:mainfrom
martincapello:other-content-types-support

Conversation

@martincapello

Copy link
Copy Markdown
Member

This PR adds the possibility to post different types of content.

Comment thread src/Visit.php Outdated
Comment on lines +195 to +198
// If no Status header is sent, assume status code 200.
else {
$this->status_code = 200;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it right? Shouldn't HTTP response always include a status code?

@martincapello martincapello Mar 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The issue here was that php-cgi doesn't add the Status header for 200 responses:
https://stackoverflow.com/a/79757436

I didn't try the suggestion of enabling "cgi.nph" because the docs says:

cgi.nph bool
If cgi.nph is enabled it will force cgi to always sent Status: 200 with every request.

So I thought it would return 200 despite the response code set...but I'll give a try and see.

@martincapello martincapello Mar 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It works as well using the cgi.nph directive. Do you prefer using it instead of this "else"?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I'd prefer cgi.nph just in case if the process fails.

@dacap

dacap commented Mar 17, 2026

Copy link
Copy Markdown
Member

LGTM, but I'd review that status code thing just in case.

@dacap dacap assigned martincapello and unassigned dacap Mar 17, 2026
@martincapello martincapello assigned dacap and unassigned martincapello Mar 17, 2026
@martincapello
martincapello force-pushed the other-content-types-support branch from 8dec359 to ba4d42a Compare March 17, 2026 16:48
@dacap

dacap commented Mar 18, 2026

Copy link
Copy Markdown
Member

I saw that you've added cgi.nph=1 but keep the $this->status_code = 200;, we should remove that line.

@dacap dacap assigned martincapello and unassigned dacap Mar 18, 2026
@martincapello
martincapello force-pushed the other-content-types-support branch from ba4d42a to 77f4c42 Compare March 18, 2026 11:59
@martincapello martincapello assigned dacap and unassigned martincapello Mar 18, 2026
@dacap
dacap merged commit 77f4c42 into igarastudio:main Mar 18, 2026
1 check passed
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