Amphtml: [amp-story-player] some valid story don't renter when using amp-cache="cdn.ampproject.org"

Created on 7 Dec 2020  路  4Comments  路  Source: ampproject/amphtml

What's the issue?

In amp-story-player when I use attribute amp-cache with value cdn.ampproject.org some stories are generating wrong cache URL. Story doesn't loads.

for example if I put https://arts.visualstories.com/quizzes/know-your-art-a-trivia-on-renaissance-art this story URL in story player when it renders the player it should generate
https://arts-visualstories-com.cdn.ampproject.org/v/s/arts.visualstories.com/quizzes/know-your-art-a-trivia-on-renaissance-art?amp_js_v=0.1#visibilityState=visible&origin=https%3A%2F%2Ffiddle.jshell.net&showStoryUrlInfo=0&storyPlayer=v0&cap=swipe
this URL but it's generating
https://arts-visualstories-com.cdn.ampproject.org/i/s/arts.visualstories.com/quizzes/know-your-art-a-trivia-on-renaissance-art?amp_js_v=0.1#visibilityState=visible&origin=https%3A%2F%2Ffiddle.jshell.net&showStoryUrlInfo=0&storyPlayer=v0&cap=swipe

See the difference is /v/ in first cache URL and /i/ in second.

How do we reproduce the issue?

https://jsfiddle.net/0ht9xrgk/

Click on the URL and preview.

What browsers are affected?

All browsers

Which AMP version is affected?

-

Bug stories

Most helpful comment

Thanks for filing this @saurabh230197 and pointing out the difference in the URLs. That's very helpful :)

This bug appears to be coming from amp-toolbox-cache-url.
In this case the URL ends in art, so the caching url generator thinks it's an image link and appends /i instead of /v

This is a bug caused by the URL ending in a string included in imageExtensions.

In amp-story-player.impl.js this originates from maybeGetCacheUrl_ which calls createCacheUrl
which calls _getResourcePath and is incorrectly thinking the path is an image at isPathNameAnImage.

We will need to improve the URL checking.

cc @Enriqe

All 4 comments

Thanks for filing this @saurabh230197 and pointing out the difference in the URLs. That's very helpful :)

This bug appears to be coming from amp-toolbox-cache-url.
In this case the URL ends in art, so the caching url generator thinks it's an image link and appends /i instead of /v

This is a bug caused by the URL ending in a string included in imageExtensions.

In amp-story-player.impl.js this originates from maybeGetCacheUrl_ which calls createCacheUrl
which calls _getResourcePath and is incorrectly thinking the path is an image at isPathNameAnImage.

We will need to improve the URL checking.

cc @Enriqe

@processprocess Is it deployed to production? I'm still getting same issue.

@saurabh230197 This fix has been merged but it wont be in production until early January due to the holiday release freeze.

In the meantime we recommend working around this by disabling amp-cache or altering the URL so that it does not end in art.

Was this page helpful?
0 / 5 - 0 ratings