Skip to content

Conversation

hikari-no-yume
Copy link
Contributor

Also implements apache_response_headers(). Note that apache_request_headers() is the real name of getallheaders(), the latter is an alias.

https://bugs.php.net/bug.php?id=65917

@indeyets
Copy link
Contributor

I'm not sure it is a good idea to keep apache_ in names of functions. it's better to either keep names "generic" (getallheaders() is perfect) or use cli_http_ prefix

aliasing, if necessary can be implemented in user-land:

if (!function_exists('apache_request_headers') and function_exists('getallheaders')) {
    function apache_request_headers() { return getallheaders(); }
}

@hikari-no-yume
Copy link
Contributor Author

@indeyets That battle has already been fought and lost. It's not just Apache which implements apache_request_headers and apache_response_headers, FastCGI supports them too.


if (ZEND_NUM_ARGS() > 0) {
WRONG_PARAM_COUNT;
}
Copy link
Member

Choose a reason for hiding this comment

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

This should use zppn:

if (zend_parse_parameters_none() == FAILURE) {
    return;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, right. Will fix.

…in...)

- Implemented apache_request_headers() and getallheaders() alias in CLI server
- Implemented apache_response_headers() in CLI server using FastCGI code
@yohgaki
Copy link
Contributor

yohgaki commented Oct 23, 2013

FastCGI should rename the function and keep consistency for function names (as discussed in internals ML)
I suggest

http_response_headers()
http_request_headers()

for all SAPIs that support getting headers. I may add aliases for other SAPIs, so please use generic function name.

@laruence
Copy link
Member

I agree with @yohgaki

@hikari-no-yume
Copy link
Contributor Author

Closed and re-opened new one from here: #528 - I'm going to try and get it into 5.5 instead, since it's not strictly a new feature.

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.

5 participants