Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#29427 closed defect (bug) (fixed)

Adjust oEmbed regex for Slideshare

Reported by: latz's profile Latz Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.5
Component: Embeds Keywords: has-patch needs-testing
Focuses: Cc:

Description

The current regex in class-oembed.php for slideshare assumes that the URL is "www.slideshare.net" or "slideshare.net".
Accessing slideshare from Germany I experienced that the URL is "de.slideshare.net". I assume there are others for various languages.

This patch changes
'#https?://(www\.)?slideshare\.net/.*#i'
to
'#https?://(.+\.)?slideshare\.net/.*#i'

so that these URLs permitted to be embedded.

Attachments (2)

29427.diff (1.2 KB) - added by Latz 10 years ago.
29427.1.diff (1.2 KB) - added by Latz 10 years ago.
Make new regex less greedy.

Download all attachments as: .zip

Change History (11)

@Latz
10 years ago

#1 @Latz
10 years ago

  • Summary changed from Adjust oEmbed reges for Slideshare to Adjust oEmbed regex for Slideshare

#2 follow-up: @johnbillion
10 years ago

  • Keywords has-patch needs-testing added

There is indeed a language switcher at the bottom of slideshare.net which changes the subdomain.

I think that regex needs to be tightened up a little. It a cursory glance, it looks like it'll be greedy.

#https?://([a-z]+\.)?slideshare\.net/.*#i will probably suffice.

@Latz
10 years ago

Make new regex less greedy.

#3 in reply to: ↑ 2 @Latz
10 years ago

I think that regex needs to be tightened up a little. It a cursory glance, it looks like it'll be greedy.

#https?://([a-z]+\.)?slideshare\.net/.*#i will probably suffice.

Yeah, you're right, I tend to make regexes too greedy. I adjusted the patch.

#4 @helen
10 years ago

  • Version changed from trunk to 3.5

This ticket was mentioned in IRC in #wordpress-dev by Latz. View the logs.


10 years ago

#6 @wonderboymusic
10 years ago

  • Milestone changed from Awaiting Review to 4.1

#7 @wonderboymusic
10 years ago

.+?\. would work for arbitrary subdomains, so that www3 and the like would work, will test for fun.

#8 @nacin
10 years ago

At this point, we're already trusting mostly arbitrary subdomains and we already use (.+\.) elsewhere, so we might as well stay consistent.

#9 @wonderboymusic
10 years ago

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

In 29735:

Allow arbitrary subdomains for the oEmbed endpoint for Slideshare.

Props Latz.
Fixes #29427.

Note: See TracTickets for help on using tickets.