WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 12 months ago

#39669 reopened defect (bug)

Appearance/Menu, Custom Link: bad URL value sanitation

Reported by: TRILOS Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Menus Keywords:
Focuses: Cc:

Description

When adding Custom Link with URL value
../
and saving the menu, WP changes the URL to
http://../

Referring to the parent directory is therefore impossible.

As this IMHO is a valid "URL" for referring the parent directory, there´s no need to (I guess) "sanitize" the field value into a "valid" URL (despite this, "http://../" is not even a valid URL).

Same with ../dirname/ => http://../dirname/

Change History (8)

#1 @bisyarin
2 years ago

Hello, @TRILOS! Could you write how to reproduce it step by step?

On my local server this link 'http://wp/../' transforms to 'http://wp/1011-2', where '1011' is the id of the page, but on update it is saved with a slug named after the title of a page. It's also not the behavior I expect, but simultaneously not the one you described.

#2 @TRILOS
2 years ago

  • Version changed from 4.7.1 to 4.7.2

Thank you @bisyarin for the addition.

Here comes a desription:

  1. Edit a menu in Appearance>Menus
  2. Add a "Custom Link", insert "../" into "URL" field
  3. Klick "Add to menu"
  4. Review "Menu Structure": Custom Link URL field contains "http://../".

Note: Same with ../dirname/ => http://../dirname/

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


2 years ago

#4 @TRILOS
22 months ago

  • Keywords needs-testing added

3 months without review, is this usual?
Issue is still a bug in 4.7.4 - how to change the affected version of this ticket?

#5 @welcher
21 months ago

  • Keywords needs-testing removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed
  • Version changed from 4.7.2 to 4.7.4

@TRILOS thank you for submitting this and welcome!

Please keep in mind that many of the people who contribute to WordPress Core do so both for free and in their spare time. We can't always get to tickets as quickly as we would like :)

This feels like this is an edge case and I am unsure of the practical use here. If the intention is to link to an asset on the server, then that can be accomplished by uploading said asset to the media library and using the link provided in the Custom Menu item. Is there a use-case that cannot be addressed by using an absolute url?

My biggest concern here is security. We'd need to bypass the esc_url call in the Walker class to allow these types of URLs that is in place to make sure the URL is safe.

#6 @welcher
21 months ago

  • Version 4.7.4 deleted

#7 @TRILOS
19 months ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Thank you for these informations. In the case I noted I had no media file to refer to, but a relative path: I wanted to link to a similar index page in a different language, placed in a path of e.g. /en/ instead of /de/. So if a WP component isn´t able to check this on safety, then it should be able and it is no edge case, but a essential case of handling syntacical correct URLs, isn´t it?

Last edited 19 months ago by TRILOS (previous) (diff)

#8 @SergeyBiryukov
12 months ago

  • Milestone set to Awaiting Review
Note: See TracTickets for help on using tickets.