Dnn.Platform
Dnn.Platform copied to clipboard
Ensure StandardFolderProvider always returns an absolute URL.
Fixes #5085
Includes the fixes suggested in PR #5086.
@mitchelsellers In this case, what are the next steps? Your current concerns are the reason why in my initial implementation, I placed it at a lower level, so the changes only happen at runtime.
I think this logic should only apply when there's an SSL offload header setup.
I agree with @bdukes that only doing this logic if an offload url is set would be proper.
That is a situation where content owners will expect environmental specific urls.
I would love to understand this scenario better though because I am confused a bit.
Say I setup an instance without SSL at http://mysite.com Then I setup any kind of thing that uses SLL offloading (say a reverse proxy for local test site). The proxy will take in external requests https://mysite.com and forward those requests to http://mysite.com The file URL would be relative like /Portals/0/files/somefile.jpg The client browser would have no clue about the inner workings of the infrastructure and that would translate to using https.
So I fail to understand how an absolute URL helps here as I was kind of expecting the exact opposite as the proper way. I remember we actually had some issues in the Persona bar with absolute links to some resource files which made some functions unusable with ssl-offloading...
There is a very, very unique situation where the SSL Offload is done using a DIFFERENT URL rather than the actual public/normal URL.
There is a very, very unique situation where the SSL Offload is done using a DIFFERENT URL rather than the actual public/normal URL.
Exactly the source of my confusion, being relative, it does not include the admin part in it right? So relative should work, unless I am missing something, I don't understand...
I will post a more extended explanation in the following hours.
We did a bit of digging on our end. The URL(line 175 in FileServerHandler) that leaves the DNN code remains relative before it is passed to Response.Redirect at line 190 in FileServerHandler.
As you can see, at line 2394 this will force the URL to be converted to an Absolute URL based on a configuration setting found in system.web/httpRuntime. This for DNN seems to default to true, possibly from the days when the RFC2616 which required the Location header to be an absolute URI. In 2014 it was replaced by RFC7231 which states that relative URIs in the Location header are now permitted.
Upon setting the "useFullyQualifiedRedirectUrl" to false, the URL remained relative in the Location header, and the request no longer changed from HTTPS to HTTP. While simply turning this option off would fix the issues mentioned, I still think that returning an absolute URL for the file would be a better option, as this would eliminate any confusion and possibility for such an issue to occur in the future.
Thanks @rhaiamz, that clarification helps a lot.
As @valadas pointed out, using a relative URL here seems most correct, so if we could preserve the relative URL, that would resolve the issue completely from my perspective.
I would suggest that the PR add a config transform to set useFullyQualifiedRedirectUrl to false. The only issue noted in the docs for that setting states "Some browsers might have problems loading pages in cookieless sessions when this value is false."
My assumption is that "some browsers" includes old browsers we don't support, and we don't support cookieless sessions, either AFAIK, so this seems like a safe change to me.
Thanks, that explanation helps me understand now
As discussed, this PR cannot be merged this way as it would produce too many side-effects.