-
Notifications
You must be signed in to change notification settings - Fork 25
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
Fix bugs for a multisite environment. #15
base: develop
Are you sure you want to change the base?
Conversation
Use get_home_url to creat the rest URI. Use apache_request_header in case $_SERVER didn't retrieve it.
@@ -80,7 +80,7 @@ public static function get_rest_uri() { | |||
$prefix = rest_get_url_prefix(); | |||
} | |||
|
|||
return sprintf( '/%s/%s/%s', $prefix, self::_NAMESPACE_, self::_REST_BASE_ ); | |||
return sprintf( '%s/%s/%s/%s', get_home_url(), $prefix, self::_NAMESPACE_, self::_REST_BASE_ ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why add the home url here? This will break the tests and would require some additional modifications.
@@ -762,6 +762,14 @@ public function get_auth_header() { | |||
$header = sanitize_text_field( $_SERVER['REDIRECT_HTTP_AUTHORIZATION'] ); | |||
} | |||
|
|||
//Maybe apache? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole block of code does not adhere to coding standards and is causing phpcs to fail the build. Please update it when you have a chance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it would make more sense to check for the missing header at the same time as the check for the function.
// Fallback for Apache request headers.
if ( ! $header && function_exists( 'apache_request_headers' ) ) {
$headers = apache_request_headers();
if ( is_array( $headers ) && ! empty( $headers[ 'Authorization' ] ) ) {
$header = sanitize_text_field( $headers[ 'Authorization' ] );
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't have anything to do with Multisite though. Can you describe the problem you're having with Multisite so I can understand why you are changing the rest endpoint?
The get_home_url is the change I did to allow your plugin to work on my
environment. Without it, the script can't figure the exact URL for the site
where it have to ask for the information. I'm using a multisite with
directories, not subdomains if this can help to clarify.
I'm using apache locally, and the information in the $_SERVER array didn't
get the authorization information. It requires to use
apache_request_headers to get this data.
These are the changes I made. Hope it clarifies any doubt you have.
Chears.
El vie., 5 jul. 2019 a las 17:12, Derek Herman (<[email protected]>)
escribió:
… ***@***.**** commented on this pull request.
------------------------------
In wp-includes/rest-api/auth/class-wp-rest-token.php
<#15 (comment)>:
> @@ -762,6 +762,14 @@ public function get_auth_header() {
$header = sanitize_text_field( $_SERVER['REDIRECT_HTTP_AUTHORIZATION'] );
}
+ //Maybe apache?
This doesn't have anything to do with Multisite though. Can you describe
the problem you're having with Multisite so I can understand why you are
changing the rest endpoint?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#15?email_source=notifications&email_token=ABQPMPNYNBJ5ZCD3QKSIP5LP57BMZA5CNFSM4HUUM6X2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB5UU4VY#discussion_r300804399>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABQPMPPVVVGYKP5AVJEIIMTP57BMZANCNFSM4HUUM6XQ>
.
--
Juan Sebastián Echeverry
www.sebaxtian.com
|
This PR will fix the issue #19 , it's directly related. |
@valendesigns hello
Although it works, it would certainly break the tests because of the hardcoded uris in
I was wondering if would be a good idea to build dynamically the uris in the test function. |
sorry guys for using this ticket for something totally unrelated ... @baxtian the comment box at http://www.sebaxtian.com/acerca-de/schreikasten does not work any longer ( Below the comment that i was going to post there Hi sebaxtian, thank you for the plugin. Would you consider uploading the source code to github / gitlab for more collaboration? People can send in fixes for the code as pull requests and make bug reports in separate tickets. We only have the release SVN, which is not for development, at https://plugins.trac.wordpress.org/browser/schreikasten/ I would also be very interested in all the ideas that you have for improvements to a shoutbox (possibly as new implementation instead of new version). You write:
If you can spare the time a full list of possible improvements would be very appreciated (perhaps in a github ticket / wiki). You can contact me at [email protected] so we don't have to use this ticket for communication. |
Use get_home_url to creat the rest URI.
Use apache_request_header in case $_SERVER didn't retrieve it.