Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

When determining whether we're a local file request, see if we have a valid host before using the window's location. #1951

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Contributor

GlennAustin commented Jun 24, 2013

Before checking whether the request is local, we should first check to see if the original request had a host before assuming that we need to use the window's host.

cappbot commented Jun 24, 2013

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

Contributor

ahankinson commented Jun 26, 2013

Thanks for you contribution. For future contributions, could you please make sure you follow our commit message guidelines? You can find them in CONTRIBUTING.md.

Fixes #1948

-#new
+bug
+Foundation
+#accepted

cappbot commented Jun 26, 2013

Milestone: Someday. Labels: #accepted, Foundation, bug. What's next? A reviewer should examine this issue.

Contributor

aparajita commented Jul 20, 2013

Hmmm... I'm at a loss to understand how a URL with http or https protocol could ever NOT be considered a non-local request. If the protocol of the current page is file:, the browser may disallow an http request, but that will cause the request to fail completely, I don't see why it should be considered a local request.

@boucher or @aljungberg, do you know of any case where the browser will magically transform an http request into a file request?

Member

boucher commented Jul 21, 2013

I think the more confusing issue is what is the _isLocalFileConnection
branch even trying to accomplish. The logic itself seems to be just any
request made from a Cappuccino app loaded from a 'local' path -- and goes
out of its way to work in titanium (and probably NativeHost, though I can't
recall).

On Sat, Jul 20, 2013 at 10:25 AM, aparajita notifications@github.comwrote:

Hmmm... I'm at a loss to understand how a URL with http or https protocol
could ever NOT be considered a non-local request. If the protocol of the
current page is file:, the browser may disallow an http request, but
that will cause the request to fail completely, I don't see why it should
be considered a local request.

@boucher https://github.com/boucher or @aljungberghttps://github.com/aljungberg,
do you know of any case where the browser will magically transform an http
request into a file request?


Reply to this email directly or view it on GitHubhttps://github.com/cappuccino/cappuccino/pull/1951#issuecomment-21297088
.

Contributor

aparajita commented Jul 21, 2013

The purpose of _isLocalFileConnection is to determine whether to pass a plain CPURLResponse or CPHTTPURLResponse when calling the delegate method connection:didReceiveResponse:. So what you are saying is that NativeHost and Titanium use the http protocol to access local files, and the response is not an http response? Because if it is an http response, I don't see the harm in sending a CPHTTPURLResponse to the delegate, even though the request is for a local file.

Contributor

GlennAustin commented Jul 23, 2013

From my reading of the sources, it looked like there could have been requests built using just a path and assuming that the protocol and host would be picked up from the current address, very similar to an aLink with an href="/path/to/new/resource" -- with no protocol or hostname, just a path to a resource using the same protocol and host as the web page.

Member

boucher commented Jul 23, 2013

So, you are specifically trying to change the behavior in the situation of:

[CPURLConnection connectionWithRequest:[CPURLRequest requestWithURL:"apple.com/foo"] ...]

is that right? Basically, I'd argue that the current behavior is correct, because any URL without a scheme is assumed to be a relative URL (and if you made an on the page with that url it would treat it as a path in the current domain). The only exception I can think of here is a protocol relative URL, but this change doesn't seem like it would address that correctly to me (but I haven't verified that).

Contributor

aparajita commented Jul 23, 2013

I would still like to know why if the scheme/protocol of the request is http/https it should ever be treated as a local file connection, or why it is incorrect to return a CPHTTPURLResponse in such a case.

Contributor

aparajita commented Jul 26, 2013

When you create a request with a bare path, scheme === undefined, so your change would never be seen, since it is subordinate to (scheme === "http" || scheme === "https").

You are right to bring up the case where the URL is a bare path, but in actual fact it looks like the code should be checking the window.location if !scheme, and setting _isLocalFileConnection accordingly.

Contributor

aparajita commented Jul 26, 2013

@boucher, I think the existing code is wrong. Why would we want to deny the user receiving a CPHTTPResponse when the request is http/https? Why does it matter if the local protocol is file:, the request is still http.

If I am running locally using file:, there is nothing to prevent me from making http/https requests as long as the server supports CORS and my origin/headers. In that case, if I am expecting headers in the response, with the current code I won't get them, which will cause my code to break. Is that really the behavior we want?

It seems to me the only thing that matters is the protocol of the request, not the protocol of the page.

Contributor

GlennAustin commented Jul 12, 2017

Old, things are probably a lot different now so I'm just going to close this.

@GlennAustin GlennAustin deleted the GlennAustin:gla/CPURLConnection branch Jul 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment