search

keithjgrant / omnibear

A Micropub browser extension
MIT License
36 stars 8 forks source link

Does not resolve relative URLs. #39

Closed Zegnat closed 6 years ago

Zegnat commented 6 years ago

The following endpoint LINK will trip Omnibear:

<link rel="authorization_endpoint" href="/test">

The user will get a page saying:

Firefox can't find the file at /path/to/omnibear/dist/test?me=http://example.com&client_id=https://omnibear.com&redirect_uri=https://omnibear.com/auth/success/&response_type=code&scope=post create delete update&state=very-secret-omnibear-state.

Zegnat commented 6 years ago

On cursory glance, this may be an issue with upstream micropub-helper (paging @grantcodes) or even one of their upstreams: the rel parsing in the used microformats parser.

Maybe glennjones/microformat-shiv#33? Not sure how much glennjones/microformat-node is used by upstream.

sknebel commented 6 years ago

This appears to be a bug in micropub-helper. I should have the necessary code in another project, I will work on a fix and submit it.

grantcodes commented 6 years ago

Oh I think I know how to fix that, the microformats libraries have an option for a baseUrl which should convert relative urls.

So I think if I grab the me value and strip any trailing paths that should get the base URL.

Zegnat commented 6 years ago

the microformats libraries have an option for a baseUrl which should convert relative urls.

Yeah, that seems to be the issue.

if I grab the me value and strip any trailing paths that should get the base URL.

Not necessarily, there might be a BASE element on the page that you have to use instead. URL resolution can be a little tricky.

grantcodes commented 6 years ago

Ok well I have updated micropub-helper to at least pass the me value. I have no idea how it will handle BASE elements at the moment, I would hope that's something the microformats libraries would handle.

I've not published the updated micropub-helper module to npm yet as I haven't had time to test properly but GitHub is updated if anyone wants to test for me.

sknebel commented 6 years ago

@grantcodes you beat me by seconds with my PR and missed one case: the me value is not necessary the final URL the HTML was delivered from (due to redirects). See my branch https://github.com/sknebel/micropub-1/tree/fix-relative - feel free to use that code.

I agree that <base> tags should be handled by the mf2 parser. It seems like they aren't though, but that's clearly a parser bug.

grantcodes commented 6 years ago

Ok with thanks to @sknebel and @Zegnat I have updated micropub-helper to v1.3.0 which should fix this issue when merged into the plugin.

I'd also recommend updating as the new version excludes all all the polyfills needed for node so should be a much smaller filesize :)