Differential D25694
Set "preconnect" HTTP header when "security.alternate-file-domain" is set Authored by aklapper on Jun 18 2024, 10:36.
Details
When a CDN or alternate file domain is configured, reduce perceived latency by resolving DNS and establishing a connection. See https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/rel/preconnect and https://we.phorge.it/book/phorge/article/configuring_file_domain/ Closes T15859
Diff Detail
Event TimelineComment Actions Should the summary also include https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/rel/preconnect as a reference? https://developer.mozilla.org/en-US/docs/Web/Performance/dns-prefetch doesn't mention preconnect in the first few paragraphs, so it's somewhat confusing. Preconnect is only useful if a page needs to make connections to third-party domains. It's unnecessary for AphrontAjaxResponse and redirects, so adding the header to the base class AphrontResponse is excessive. What do you think of moving it to AphrontHTMLResponse?
Comment Actions What if $cdn has some evil characters? Do we have some escaping, or can it cause anything bad, or do we not care? Looks like we're including a bunch of other config as-is here, so maybe it's ok. Comment Actions
If the domain is invalid or does not host Phorge assets: Failing to load assets like CSS/images/JS as the resulting URIs are invalid. Still, if an admin sets /config/edit/security.alternate-file-domain/ in the Phorge installation they are supposed to maintain to a domain not fully under their control or to include evil characters then I'd be way more worried about having system administrators who seem to have no clue what they are doing when pushing or tweaking random knobs in a piece of software.
Comment Actions Premising that probably no need to escape headers: https://stackoverflow.com/a/5677844/3451846 But yes, probably need to escape the URI inside <...>. The example is limited to the path, but probably relevant also for the domain itself: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link#encoding_urls Comment Actions After 15 minutes of reading stuff around on the web, I think we need a little help. Waiting for answers (and waiting for downvotes lol). Comment Actions
I'm not sure it matters for now as Phorge does not seem to accept non-ascii domains anyway (with a confusing error message): [acko@fedora phorge (T15859preFetch *$|u+1)]$ ./bin/config set "security.alternate-file-domain" "https://苗条.example.com" Usage Exception: Config option 'security.alternate-file-domain' is invalid. The URI must start with 'http://' or 'https://'. [acko@fedora phorge (T15859preFetch *$|u+1)]$ ./bin/config set "security.alternate-file-domain" "https://áéexample.com" Usage Exception: Config option 'security.alternate-file-domain' is invalid. The URI must start with 'http://' or 'https://'. I could imagine something like $cdn = preg_replace_callback('/[^\x20-\x7f]/', function($match) { return rawurlencode($match[0]); }, $cdn); to work. Untested because above.
Comment Actions Move CDN check from generic AphrontResponse to more correct AphrontHTMLResponse. I confirm this still sets the HTTP Header. Comment Actions Looks like installing php-xdebug forces me to set 'xdebug.mode' to 'coverage' in 'php.ini' to pass arc unit, no matter what... Comment Actions Good catch. Phorge does not currently support these scaring esoteric domains that MAY need a bigger test plan. In fact, the utility PhutilURI is clearly not supporting "internationalized domain names" right now, as I've now described in T16048. Whenever T16048 is open or closed, Phorge demonstrated to already validate configured URI(s) before using them here, so, this patch itself is safe. Also, I've tested that creating this local alternative: http://苗条asdasasasd.phorge.localhost/ That, once encoded, it's this: http://xn--asdasasasd-mr0vl07w.phorge.localhost/ And that can be configured right now with this command: ./bin/config set security.alternate-file-domain http://xn--asdasasasd-mr0vl07w.phorge.localhost/ /etc/hosts 127.0.0.1 xn--asdasasasd-mr0vl07w.phorge.localhost <VirtualHost *:80>
ServerName phorge.localhost
ServerAlias xn--asdasasasd-mr0vl07w.phorge.localhost
DocumentRoot /var/www/phorge/webroot
RewriteEngine on
RewriteRule ^(.*)$ /index.php?__path__=$1 [B,L,QSA]
php_admin_value post_max_size 32M
</VirtualHost>Again, this is just an attempt to screw up my local installation and be aggressive here. Don't try since T16048 is not implemented. Another hypotetical problem already covered: somebody may say: «what if my Phorge is served by phorge.example.com and I've set security.alternate-file-domain? for my use case, isn't it better to avoid to generate a duplicate Link????? UHM UHM????». Answer: dear hypotetical user, this potential workflow has no sense, since that config is designed to have a different domain, or null. Therefore, this is safe, tested, approved, and probably already ready for future improvement e.g. T16048. Thaanks! Comment Actions (Plus, a domain 0 is nonsense, so the well-known PHP pitfall of if($something) is excluded here - double-slam-accept) | ||||||||||||||||||||||||||||||||||||||