WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 10 months ago

Last modified 10 months ago

#38182 closed defect (bug) (fixed)

rest_url does not support index permalinks

Reported by: kraftbj Owned by: kraftbj
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

WordPress has a little known third permalink style -- index permalinks. These are example.com/index.php/2016/03/example-post/ style permalinks for sites that can not enable mod_rewrite for whatever reason.

When using index permalinks, the rest_url function returns example.com/wp-json/ which will 404 on these sites.

Attachments (2)

38182.diff (1.3 KB) - added by kraftbj 11 months ago.
functional but ugly way to have rest_url() work with index permalinks
38182.2.diff (2.4 KB) - added by kraftbj 10 months ago.
same as previous, adding unit tests

Download all attachments as: .zip

Change History (15)

@kraftbj
11 months ago

functional but ugly way to have rest_url() work with index permalinks

#1 @jorbin
11 months ago

  • Milestone changed from Awaiting Review to 4.7

#2 @kraftbj
11 months ago

  • Milestone changed from 4.7 to Awaiting Review

I've attached an ugly patch to resolve this that:

  1. Provides the index permalink style when that's being used.
  2. Adds the rest end points to the index.php permalink style.

To do this *right*, I'm thinking we should figure out:

  1. A better way to check for index permalinks. Calling the global is ugly. Add helper functions.
  2. Why is the REST API rewrites added separately? Should we integrate them into the actual rewrite class?

38182.diff may be okay for a maintenance release.

#3 @kraftbj
11 months ago

  • Milestone changed from Awaiting Review to 4.7

Resetting the version. Saved my comment after @jorbin moved it to 4.7.

#4 @georgestephanis
11 months ago

For anyone hitting this problem before it gets fixed by core, you can apply this fix in a mu-plugins file or your theme's index.php to get it working right now:

add_filter( 'rest_url_prefix', 'fix_rest_url_prefix_for_rewrites' );
function fix_rest_url_prefix_for_rewrites(){
    return 'index.php/wp-json';
});

in PHP 5.3+.

Last edited 11 months ago by georgestephanis (previous) (diff)

#5 @rachelbaker
11 months ago

  • Keywords has-patch needs-unit-tests added

@kraftbj Thank you for the ticket and patch. Would be ideal to have some unit tests to make sure we don't break this in the future.

#6 @kraftbj
11 months ago

  • Owner set to kraftbj
  • Status changed from new to assigned

A quick note to say that I'm planning on working up the unit tests, but haven't had the available time yet.

@kraftbj
10 months ago

same as previous, adding unit tests

#7 @kraftbj
10 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

I added/reworked the unit tests to use the phpunit helper function for setting permalinks to ensure the various pieces of the $wp_rewrite class are refreshed.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


10 months ago

#9 @rachelbaker
10 months ago

  • Keywords commit added

#10 @kraftbj
10 months ago

  • Version set to 4.4

#11 @rachelbaker
10 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38790:

REST API: Support sites with index-style permalinks in get_rest_url().

Support the index-style permalinks (http://example.com/index.php/postName) when registering the REST API rewrite rules and within the get_rest_url() function. This allows sites that do not have mod_rewrite support to have almost pretty urls and have access to their REST API endpoints.

Props kraftbj.
Fixes #38182.

#12 @dd32
10 months ago

@rachelbaker @kraftbj Is there any reason why extra rewrites are needed here?

For example, it could be done like so:

add_rewrite_rule( '^' . ( $wp_rewrite->index ? $wp_rewrite->index . '/' : '' ) . rest_get_url_prefix() . '/?$','index.php?rest_route=/','top' );

Sites which use that format would have something like this at present:

array (size=142)
  '^wp-json/?$' => string 'index.php?rest_route=/' (length=22)
  '^wp-json/(.*)?' => string 'index.php?rest_route=/$matches[1]' (length=33)
  '^index.php/wp-json/?$' => string 'index.php?rest_route=/' (length=22)
  '^index.php/wp-json/(.*)?' => string 'index.php?rest_route=/$matches[1]' (length=33)
  'index.php/category/(.+?)/feed/(feed|rdf|rss|rss2|atom)/?$' => string 'index.php?category_name=$matches[1]&feed=$matches[2]' (length=52)

While I'm looking at this, any particular reason why the second rule is an optional match? Surely ^wp-json/(.+) makes more sense considering the first catches an empty request.. and as all rewrite rules are anchored to the start, the ^ carrot isn't even needed (It ultimately means PCRE is matching on #^^wp-json/...)

#13 @kraftbj
10 months ago

@dd32 We would want to make sure sites not using index permalinks still work without the extra rule. $wp_rewrite->index would always be set too, so need to check for $wp_rewrite->using_index_permalinks() before adding conditional support (which probably should have a better helpful function per https://core.trac.wordpress.org/ticket/38182#comment:2 ).

I agree it looks like we can update the rewrite rule additions to be smarter.

Note: See TracTickets for help on using tickets.