Skip to:
Content

BuddyPress.org

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#6353 closed defect (bug) (fixed)

BuddyPress navigation appends a trailing slash to the link in a subnav item when link is forced

Reported by: pareshradadiya Owned by: boonebgorges
Milestone: 2.3 Priority: normal
Severity: normal Version: 2.2.1
Component: Core Keywords: has-patch
Cc:

Description

As described in summary link has a trailing slash http://example.com/foo/bar/blah/?action=edit&id=30/. So when i do

$_REQUEST['id']


it give me 30/. Url should be like this http://example.com/foo/bar/blah/?action=edit&id=30

Attachments (1)

link-trailingslash.diff (1.7 KB) - added by pareshradadiya 4 years ago.
Path and php unit testcases

Download all attachments as: .zip

Change History (8)

@pareshradadiya
4 years ago

Path and php unit testcases

#1 @pareshradadiya
4 years ago

  • Summary changed from BuddyPress navigation appends a trailing slash to the link in a subnav item when like is forced to BuddyPress navigation appends a trailing slash to the link in a subnav item when link is forced

#2 @boonebgorges
4 years ago

  • Milestone changed from Awaiting Review to 2.3

Thanks for the report and for the patch. I think this is a good call - when plugin registers its own link, we should leave it up to the plugin author to ensure that it's trailingslashed. (Side note: this is a good reminder that trailingslashit() is not a very sophisticated function. A "trailingslashit-for-URLs" would be properly sensitive to query params.)

#3 @boonebgorges
4 years ago

In 9700:

Add some tests for trailingslashit() behavior in bp_core_new_subnav_item().

See #6353.

#4 @boonebgorges
4 years ago

In 9701:

Don't trailingslash the href of subnav items if the 'link' was provided explicitly in bp_core_new_subnav_item().

trailingslashit() is not smart enough to detect links with query params, so
that links can be improperly trailingslashed in some cases. More generally,
it's not the case that all links should be trailingslashed (in particular,
those that do not lead to WP/BP pages), so plugin authors should be responsible
for trailingslashing on their own.

Props pareshradadiya.
See #6353.

#5 @boonebgorges
4 years ago

Also, thanks for the security enhancement of esc_url_raw(), though in this case I believe we should be using esc_url(), because we want to encode URL entities.

#6 @boonebgorges
4 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 9702:

Improved attribute escaping when outputting subnav link items.

Props pareshradadiya.
Fixes #6353.

#7 @pareshradadiya
4 years ago

Yep. I agree with esc_url(). thanks for your correction.

Note: See TracTickets for help on using tickets.