Adding redirect count tests in case of cross origin redirections #4102
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| <!doctype html> | ||
| <html> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>Fetch in worker: redirection loop with cross-origin redirections</title> | ||
| <meta name="help" href="https://fetch.spec.whatwg.org/#http-network-or-cache-fetch"> | ||
| <script src="/resources/testharness.js"></script> | ||
| <script src="/resources/testharnessreport.js"></script> | ||
| </head> | ||
| <body> | ||
| <script> | ||
| fetch_tests_from_worker(new Worker("redirect-count-cross-origin.js")); | ||
| </script> | ||
| </body> | ||
| </html> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| <!doctype html> | ||
| <html> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>Fetch: redirection loop, then redirection to a cross origin</title> | ||
| <meta name="help" href="https://fetch.spec.whatwg.org/#http-network-or-cache-fetch"> | ||
| <script src="/resources/testharness.js"></script> | ||
| <script src="/resources/testharnessreport.js"></script> | ||
| </head> | ||
| <body> | ||
| <script src="/common/utils.js"></script> | ||
| <script src="/common/get-host-info.sub.js"></script> | ||
| <script src="../resources/utils.js"></script> | ||
| <script src="redirect-count-cross-origin.js"></script> | ||
| </body> | ||
| </html> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| if (this.document === undefined) { | ||
| importScripts("/resources/testharness.js"); | ||
| importScripts("../resources/utils.js"); | ||
| importScripts("/common/utils.js"); | ||
| importScripts("/common/get-host-info.sub.js"); | ||
| } | ||
|
|
||
| function redirectCount(desc, redirectUrl, redirectLocation, redirectStatus, maxCount1, maxCount2, requirePreflight) | ||
| { | ||
| var allow_headers = ""; | ||
| var requestInit = {"redirect": "follow"}; | ||
| if (requirePreflight) { | ||
| requestInit.headers = [["x-test", "test"]]; | ||
| allow_headers = "&allow_headers=x-test"; | ||
| } | ||
|
|
||
| var uuid_token = token(); | ||
|
|
||
| var urlParameters = "?token=" + uuid_token + "&max_age=0"; | ||
| urlParameters += "&redirect_status=" + redirectStatus; | ||
| urlParameters += "&max_count=" + maxCount1; | ||
| urlParameters += allow_headers; | ||
| urlParameters += "&location=" + encodeURIComponent(redirectLocation + "?token=" + uuid_token + "&max_age=0&max_count=" + (maxCount1 + maxCount2) + allow_headers); | ||
|
Member
There was a problem hiding this comment. It seems you're adding allow_headers and max_age twice. Also max_age is never used in the Python resource.
Contributor
Author
There was a problem hiding this comment. They are added to the first looped URL and the second looped URL.
Member
There was a problem hiding this comment. Sorry, yeah, that probably works.
Member
There was a problem hiding this comment. But max_age should probably still be removed. |
||
|
|
||
| var maxCount = maxCount1 + maxCount2; | ||
|
Member
There was a problem hiding this comment. This should move up so you don't need to do this addition twice. |
||
| var url = redirectUrl; | ||
|
Member
There was a problem hiding this comment. This seems rather redundant renaming? |
||
| promise_test(function(test) { | ||
| return fetch(RESOURCES_DIR + "clean-stash.py?token=" + uuid_token).then(function(resp) { | ||
| assert_equals(resp.status, 200, "Clean stash response's status is 200"); | ||
|
Member
There was a problem hiding this comment. It's always 200, no? The only difference seems to be whether the contents are "1" or "0".
Contributor
Author
There was a problem hiding this comment. It is just a safety check that the overall test env is correct. |
||
|
|
||
| if (maxCount > 20) | ||
| return promise_rejects(test, new TypeError(), fetch(url + urlParameters, requestInit)); | ||
|
|
||
| return fetch(url + urlParameters, requestInit).then(function(resp) { | ||
| assert_equals(resp.status, 200, "Response's status is 200"); | ||
| return resp.text(); | ||
| }).then(function(body) { | ||
| assert_equals(body, maxCount.toString(), "Redirected " + maxCount + " times"); | ||
| }); | ||
| }); | ||
| }, desc); | ||
| } | ||
|
|
||
| var redirUrl = "/fetch/api/resources/redirect-count.py"; | ||
| var crossOriginRedirUrl = get_host_info().HTTP_ORIGIN_WITH_DIFFERENT_PORT + redirUrl; | ||
|
|
||
| redirectCount("Redirect 20 times, last redirection to cross-origin", redirUrl, crossOriginRedirUrl, 301, 19, 1); | ||
| redirectCount("Redirect 21 times, last redirection to cross-origin", redirUrl, crossOriginRedirUrl, 301, 20, 1); | ||
|
|
||
| redirectCount("Redirect 20 times, going to cross-origin after 10", redirUrl, crossOriginRedirUrl, 302, 10, 10); | ||
| redirectCount("Redirect 21 times, going to cross-origin after 10", redirUrl, crossOriginRedirUrl, 302, 10, 11); | ||
|
|
||
| redirectCount("Redirect 20 times, last redirection to cross-origin with preflight", redirUrl, crossOriginRedirUrl, 301, 19, 1, true); | ||
| redirectCount("Redirect 21 times, last redirection to cross-origin with preflight", redirUrl, crossOriginRedirUrl, 301, 20, 1, true); | ||
|
|
||
| redirectCount("Redirect 20 times, going to cross-origin after 10 with preflight", redirUrl, crossOriginRedirUrl, 303, 10, 10, true); | ||
| redirectCount("Redirect 21 times, going to cross-origin after 10 with preflight", redirUrl, crossOriginRedirUrl, 303, 10, 11, true); | ||
|
|
||
|
|
||
| done(); | ||
|
Member
There was a problem hiding this comment. Can you explain this call? I never quite understood that in the Fetch API tests.
Contributor
Author
There was a problem hiding this comment. Isn't it needed for worker-environment tests? |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| from urllib import urlencode | ||
| from urlparse import urlparse | ||
|
|
||
| def main(request, response): | ||
| stashed_data = {'count': 0, 'preflight': "0"} | ||
| status = 302 | ||
| headers = [("Content-Type", "text/plain"), | ||
| ("Cache-Control", "no-cache"), | ||
| ("Pragma", "no-cache"), | ||
| ("Access-Control-Allow-Origin", "*")] | ||
| token = None | ||
|
|
||
| if "token" in request.GET: | ||
| token = request.GET.first("token") | ||
| data = request.server.stash.take(token) | ||
| if data: | ||
| stashed_data = data | ||
|
|
||
| if request.method == "OPTIONS": | ||
| if "allow_headers" in request.GET: | ||
| headers.append(("Access-Control-Allow-Headers", request.GET['allow_headers'])) | ||
| stashed_data['preflight'] = "1" | ||
| #Preflight is not redirected: return 200 | ||
| if not "redirect_preflight" in request.GET: | ||
|
Member
There was a problem hiding this comment. Is redirect_preflight ever passed?
Contributor
Author
There was a problem hiding this comment. Do not remember. I can check to remove it. |
||
| if token: | ||
| request.server.stash.put(request.GET.first("token"), stashed_data) | ||
| return 200, headers, "" | ||
|
|
||
| if "redirect_status" in request.GET: | ||
| status = int(request.GET['redirect_status']) | ||
|
|
||
| stashed_data['count'] += 1 | ||
|
|
||
| is_final_redirection = False | ||
| if token: | ||
| request.server.stash.put(request.GET.first("token"), stashed_data) | ||
| if "max_count" in request.GET: | ||
| max_count = int(request.GET['max_count']) | ||
| #stop redirecting and return count | ||
| if stashed_data['count'] > max_count: | ||
| if not "location" in request.GET: | ||
| # -1 because the last is not a redirection | ||
| return 200, headers, str(stashed_data['count'] - 1) | ||
| # After max_count, let's go to the final location. | ||
| headers.append(("Location", request.GET['location'])) | ||
| is_final_redirection = True | ||
|
|
||
| if not is_final_redirection: | ||
| # Let's redirect to the same URL except for count parameter | ||
| url = request.url.split("?")[0]; | ||
| scheme = urlparse(request.url).scheme | ||
| if scheme == "" or scheme == "http" or scheme == "https": | ||
| #keep url parameters in location | ||
| url_parameters = {} | ||
| for item in request.GET.items(): | ||
| if item[0] != "count": | ||
| url_parameters[item[0]] = item[1][0] | ||
| #make sure location changes during redirection loop | ||
| url += "?count=" + str(stashed_data['count']) + "&" + urlencode(url_parameters) | ||
| headers.append(("Location", url)) | ||
|
|
||
| return status, headers, "" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redirectUrl and redirectLocation could probably be removed since they're always the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They should not always be the same.
The principle of the test is that we will redirect macCount1 times on redirectUrl.
On the last redirect to redirectUrl, we should be redirected to redirectLocation (bad name probably)
And we again are redirected macCount2 on redirectLocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you always pass the same values as arguments here. There is no method call to redirectCount where they have a different value as far as I can tell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redirect-count.py is expected to do that (is_final_redirection check or something like that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you're misunderstanding my feedback. My point was that they don't have to be arguments to the
redirectCount()function as theredirectCount()function is never invoked with different values for them. Surelyredirect-count.pyis not invoking JavaScript?