[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #25985 [Obfuscation/Snowflake]: Snowflake rendezvous using AMP cache
#25985: Snowflake rendezvous using AMP cache
-----------------------------------+--------------------------------
Reporter: twim | Owner: (none)
Type: project | Status: needs_revision
Priority: Medium | Milestone:
Component: Obfuscation/Snowflake | Version:
Severity: Normal | Resolution:
Keywords: | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
-----------------------------------+--------------------------------
Comment (by twim):
Replying to [comment:24 dcf]:
Thanks for the review!
For now I've fixed amper-related issues you brought up.
> (On further reflection, what I was trying to do wouldn't have worked
anyway, because the AMP client code would have sent a URL path of
`/c/s/.../amp/client/...`, not `/amp/client/...`, but I could have worked
around that for testing.) It looks like the cause is
[https://github.com/nogoegst/amper/blob/6dbbc42eb44c1f389591c83a268524d5eceed1d1/client.go#L55
here in amper/client.go], with a hardcoded `"https"`. Looking at that, I
also noticed a
[https://github.com/nogoegst/amper/blob/6dbbc42eb44c1f389591c83a268524d5eceed1d1/client.go#L23
CDNDomain = "cdn.ampproject.org"]. I have to say, it skeeves me a bit to
see hardcoded constants like that in a library; it seems like the kind of
thing that belongs in a configuration file.
Thanks to your suggestion I made some constants configurable. One can now
specify Client.Scheme and Client.CDNDomain.
As you may guess, it was intended to work with the only AMP cache around
thus the hardcoded values.
> Here's an example of what the AMP responses look like:
> ...
> The overall AMP container encoding looks pretty reasonable. A few tiny
questions:
> * [https://www.ampproject.org/docs/getting_started/create/basic_markup
basic_markup] says the `<script async>` has to be the second child of
`<head>`, right after `<meta charset="utf-8">`.
You're right. Despite it is being accepted in validator, I moved it to the
right place to avoid a breakage.
> * The `<style>body {opacity: 0}</style><noscript><style>body {opacity:
1}</style></noscript>` doesn't match the documented
[https://www.ampproject.org/docs/reference/spec/amp-boilerplate.html AMP
boilerplate code], which is much longer. Is there a reason?
This is knowm as "old boilerlate"
(https://github.com/ampproject/amphtml/commit/0a056ca50ac8cb9ba8e5a6489baeecb5ed958556
#diff-17672f7dfc8b0583c360b45234dbf59a). The reason is that it's much
shorter than the new one. I've picked this as a tradeoff to save bandwidth
as the old boilerplate is still accepted by AMP CDN.
Now this is also configurable: set Server.UseOldAMPBoilterplate to true to
use shorter boilerplate, or get completely valid 'modern' AMP page by
default.
> * `<link rel="canonical" href="https://ampproject.org"; />`: it would be
better not to refer to any third parties. The AMP guide says this can be a
relative URL, so I would prefer one of these if they work:
> * `<link rel="canonical" href="">`
> * `<link rel="canonical" href="#">`
> * `<link rel="canonical" href="?">`
Yep, changed to `href="#"` and it works.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25985#comment:25>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs