Skip to content
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 to import Vimeo videos #8804

Open
wants to merge 7 commits into
base: 1.10.x
Choose a base branch
from

Conversation

AndreaPiovanelli
Copy link
Contributor

In reference to #8803 , added a fix to avoid the scraping of the "vimeo.com/xxxx" page (which replies with a 429 http error) by forcing the direct api call if Vimeo domain is recognized.

@sebastienros
Copy link
Member

I think this is really wrong. If Vimeo doesn't support OEmbed then it's better to have another feature dedicated to vimeo, or just drop vimeo support altogether.

@AndreaPiovanelli
Copy link
Contributor Author

I think this is really wrong. If Vimeo doesn't support OEmbed then it's better to have another feature dedicated to vimeo, or just drop vimeo support altogether.

Vimeo still supports embed (and the xml format is still the same) but it doesn't include the link to the video metadata xml in its markup anymore, so it cannot be automatically imported by directly reading the video page content.

While investigating further, we also noticed Youtube stopped working too. This is caused by the fact it blocks video output when trying to watch a video from a server (a bot protection). This is what happened when I tried to debug the Youtube issue directly from the server: I was trying to find a way to get the meta link in the page, but that's what the page shows:
image

For this reason, we worked on a patch on the code, similar to what has already been done to get Vimeo's oembed xml data: to properly get the provider, we check for the url we're redirected to and, with that information, call the right api.

Comment on lines 83 to 84
// <link rel="alternate" href="http://vimeo.com/api/oembed.xml?url=http%3A%2F%2Fvimeo.com%2F23608259" type="text/xml+oembed">

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// <link rel="alternate" href="http://vimeo.com/api/oembed.xml?url=http%3A%2F%2Fvimeo.com%2F23608259" type="text/xml+oembed">

var content = webClient.DownloadString(Server.HtmlDecode(href));
viewModel.Content = XDocument.Parse(content);
} catch {
// bubble exception
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// bubble exception
throw;

source = webClient.DownloadString(url);

viewModel.Content = XDocument.Parse(source);
} else if (youtube) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else if (youtube) {
}
else if (youtube) {

source = webClient.DownloadString(url);

viewModel.Content = XDocument.Parse(source);
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else {
}
else {

try {
var content = webClient.DownloadString(Server.HtmlDecode(href));
viewModel.Content = XDocument.Parse(content);
} catch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} catch {
}
catch {

@@ -137,8 +165,7 @@ public ActionResult IndexPOST(string folderPath, string url, string type, string
root.El("description", description);
}
Response.AddHeader("X-XSS-Protection", "0"); // Prevents Chrome from freaking out over embedded preview
}
catch {
} catch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} catch {
}
catch {

@@ -167,8 +194,7 @@ public ActionResult Import(string folderPath, string url, string document) {

if (oembed.Element("title") != null) {
part.Title = oembed.Element("title").Value;
}
else {
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else {
}
else {

@@ -213,8 +239,7 @@ public ActionResult Replace(int replaceId, string url, string document) {

if (oembed.Element("title") != null) {
replaceMedia.Title = oembed.Element("title").Value;
}
else {
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else {
}
else {

Comment on lines +270 to +279
HttpWebRequest myHttpWebRequest = (HttpWebRequest)WebRequest.Create(myUri);
// Send the request and wait for response.
HttpWebResponse myHttpWebResponse = (HttpWebResponse)myHttpWebRequest.GetResponse();

if (!myUri.Equals(myHttpWebResponse.ResponseUri)) {
myUri = myHttpWebResponse.ResponseUri;
}

// Release resources of response object.
myHttpWebResponse.Close();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
HttpWebRequest myHttpWebRequest = (HttpWebRequest)WebRequest.Create(myUri);
// Send the request and wait for response.
HttpWebResponse myHttpWebResponse = (HttpWebResponse)myHttpWebRequest.GetResponse();
if (!myUri.Equals(myHttpWebResponse.ResponseUri)) {
myUri = myHttpWebResponse.ResponseUri;
}
// Release resources of response object.
myHttpWebResponse.Close();
var myHttpWebRequest = WebRequest.Create(myUri) as HttpWebRequest;
// Send the request and wait for response.
using (var myHttpWebResponse = myHttpWebRequest.GetResponse() as HttpWebResponse) {
if (!myUri.Equals(myHttpWebResponse.ResponseUri)) {
myUri = myHttpWebResponse.ResponseUri;
}
}

@BenedekFarkas
Copy link
Member

I think this is really wrong. If Vimeo doesn't support OEmbed then it's better to have another feature dedicated to vimeo, or just drop vimeo support altogether.

Vimeo still supports embed (and the xml format is still the same) but it doesn't include the link to the video metadata xml in its markup anymore, so it cannot be automatically imported by directly reading the video page content.

While investigating further, we also noticed Youtube stopped working too. This is caused by the fact it blocks video output when trying to watch a video from a server (a bot protection). This is what happened when I tried to debug the Youtube issue directly from the server: I was trying to find a way to get the meta link in the page, but that's what the page shows: image

For this reason, we worked on a patch on the code, similar to what has already been done to get Vimeo's oembed xml data: to properly get the provider, we check for the url we're redirected to and, with that information, call the right api.

@sebastienros what do you think? BTW I tested the YouTube import (but couldn't test Vimeo), works fine.

@sebastienros
Copy link
Member

Let's keep oembed as is, it's oembed. Add some other option called youtube, another one for vimeo and they are dedicated. Or a single one called "video" which would support whatever we want. Copy as much as you want from the current oembed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants