Replacing PHP with equivalent Python CGI scripts
<rdar://problem/74399686>
Created attachment 420693 [details] Patch
Created attachment 420714 [details] Patch
Created attachment 420724 [details] Patch
Created attachment 420728 [details] Patch
Created attachment 420750 [details] Patch
Created attachment 420792 [details] Patch
Comment on attachment 420792 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420792&action=review > LayoutTests/http/tests/misc/resources/charset-sniffer-end-sniffing.py:15 > +for i in range(0, 10000): The usual idiom when you have a variable that you need to complete a statement but that you are not actually going to use is to use '_' > LayoutTests/http/tests/misc/resources/check-test-file.py:28 > + 'PASS: File upload was successful\n' Is there any "error" equivalent in Python? This code isn't exactly the same, although it's the same in the successful case, so that's probably sufficient. > LayoutTests/http/tests/misc/resources/check-unnamed-file-included-in-formdata.py:23 > +if data != None: Style is to use "data is not None" in this case. > LayoutTests/http/tests/misc/resources/redirect-to-http-url.py:1 > +#!/usr/bin/env python3 Is there a php file being removed here? > LayoutTests/http/tests/misc/resources/referrer-main-resource.py:6 > +referer = os.environ.get('HTTP_REFERER', '') Don't need the default empty string (since you aren't printing it) and you probably don't need the variable either and could just put the os.environ.get in the if statement > LayoutTests/http/tests/misc/resources/referrer-result.py:14 > +# formatting string Remove this comment. I suspect the issue is about the brackets in the string, and can be mitigated with double brackets, but it's not a huge issue if these stay as "prints" > LayoutTests/http/tests/misc/resources/stylesheet-bad-mime-type.py:18 > + sys.stdout.write('\r\n') I think we want the 406 status code here
Created attachment 420871 [details] Patch
Created attachment 420886 [details] Patch
Created attachment 420917 [details] Patch
Created attachment 421001 [details] Patch
Created attachment 421010 [details] Patch
Created attachment 421018 [details] Patch
Created attachment 421019 [details] Patch
Created attachment 421021 [details] Patch
Created attachment 421048 [details] Patch
(In reply to Jonathan Bedard from comment #8) > Comment on attachment 420792 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=420792&action=review > > > LayoutTests/http/tests/misc/resources/charset-sniffer-end-sniffing.py:15 > > +for i in range(0, 10000): > > The usual idiom when you have a variable that you need to complete a > statement but that you are not actually going to use is to use '_' > > > LayoutTests/http/tests/misc/resources/check-test-file.py:28 > > + 'PASS: File upload was successful\n' > > Is there any "error" equivalent in Python? This code isn't exactly the same, > although it's the same in the successful case, so that's probably sufficient. > > > LayoutTests/http/tests/misc/resources/check-unnamed-file-included-in-formdata.py:23 > > +if data != None: > > Style is to use "data is not None" in this case. > > > LayoutTests/http/tests/misc/resources/redirect-to-http-url.py:1 > > +#!/usr/bin/env python3 > > Is there a php file being removed here? > > > LayoutTests/http/tests/misc/resources/referrer-main-resource.py:6 > > +referer = os.environ.get('HTTP_REFERER', '') > > Don't need the default empty string (since you aren't printing it) and you > probably don't need the variable either and could just put the > os.environ.get in the if statement > > > LayoutTests/http/tests/misc/resources/referrer-result.py:14 > > +# formatting string > > Remove this comment. I suspect the issue is about the brackets in the > string, and can be mitigated with double brackets, but it's not a huge issue > if these stay as "prints" > > > LayoutTests/http/tests/misc/resources/stylesheet-bad-mime-type.py:18 > > + sys.stdout.write('\r\n') > > I think we want the 406 status code here Fixed in comment 17
Created attachment 421054 [details] Patch
Created attachment 421061 [details] Patch
Created attachment 421189 [details] Patch
Created attachment 421193 [details] Patch
Created attachment 421204 [details] Patch
Created attachment 421208 [details] Patch
Created attachment 421213 [details] Patch
Created attachment 421237 [details] Patch
Created attachment 421282 [details] Patch
Created attachment 421309 [details] Patch
Created attachment 421351 [details] Patch
(In reply to Chris Gambrell from comment #29) > Created attachment 421351 [details] > Patch There are still 3 PHP files remaining. Talked to Jonathan and it is beneficial to proceed with committing the changes that are complete and move on to more directories to keep progressing through the files we need to convert. This directory will be revisited later. After cq+ the bug should remain open and not resolved. LayoutTests/http/tests/misc/resources/redirect-to-http-url.php LayoutTests/http/tests/misc/resources/prefetch-purpose.php LayoutTests/http/tests/misc/resources/check-query-param.php
Comment on attachment 421351 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=421351&action=review > LayoutTests/http/tests/loading/resources/slowimage.py:1 > +#!/usr/bin/env python3 I don't see the file that this is replacing, and it looks like a duplicate? > LayoutTests/http/tests/misc/refresh-headers.py:11 > +if cache_control.lower() == 'no-cache' or pragma.lower == 'no-cache': Should be pragma.lower() > LayoutTests/http/tests/misc/resources/char-encoding-in-hidden-charset-field.py:7 > +data = ''.join(sys.stdin.readlines()) It seems wrong to read data if our request method is not a POST request > LayoutTests/http/tests/misc/resources/check-query-param.py:1 > +#!/usr/bin/env python3 I don't see the PHP file we are replacing removed. > LayoutTests/http/tests/misc/resources/dns-prefetch-control.py:10 > +sys.stdout.write('Content-Type: text/html\r\n\r\n') X-DNS-Prefetch-Control is part of the header. You are ending the header here. > LayoutTests/http/tests/misc/resources/dns-prefetch-control.py:17 > + sys.stdout.write('X-DNS-Prefetch-Control: foo\r\n\r\n') Maybe just: if value in ['on', 'off', 'foo']: sys.stdout.write('X-DNS-Prefetch-Control: off\r\n'.format(value)) > LayoutTests/http/tests/misc/resources/image-checks-for-accept.py:6 > +allimages = 'image/*' This doesn't really need be a variable. > LayoutTests/http/tests/misc/resources/webtiming-cross-origin-and-back-redirect2.php:-2 > - // 127.0.0.1 is where the test originally started. We redirected to "localhost" before this. I would keep this comment in the Python file > LayoutTests/http/tests/misc/resources/protected/protected-image.py:15 > +if !username: Use `not username`
Created attachment 421859 [details] Patch
Created attachment 421861 [details] Patch
Created attachment 421892 [details] Patch
Created attachment 421937 [details] Patch
Committed r273819: <https://commits.webkit.org/r273819> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421937 [details].
Reopening for second pass
Created attachment 424985 [details] Patch
Created attachment 424986 [details] Patch
Created attachment 424987 [details] Patch
Comment on attachment 424987 [details] Patch apply-patch is having issues processing http/tests/misc/will-send-request-returns-null-on-redirect.html's resources being converted from .php to .py. No other tests use http/tests/misc/resources/redirect-to-http-url.php so I have included the .py version without removing the .php file so that EWS can chew on the other converted files. The test file that references http/tests/misc/resources/redirect-to-http-url.py (the file that is having trouble going through apply-patch) passes locally on wk1 and wk2. platform/wk2/http/tests/misc/will-send-request-returns-null-on-redirect-expected.txt will need to be updated along with http/tests/misc/will-send-request-returns-null-on-redirect.html and http/tests/misc/will-send-request-returns-null-on-redirect-expected.txt. Once r+, I will remove the .php version and update the files in question to reflect the .py version instead of the .php and will land it manually to bypass the apply-patch issues
Comment on attachment 424987 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=424987&action=review > LayoutTests/http/tests/misc/resources/check-query-param.py:21 > +if 'HTTP_COOKIE' in os.environ: Might consider using get_cookies and request.update() > LayoutTests/http/tests/misc/resources/redirect-to-http-url.py:1 > +#!/usr/bin/env python3 Where is the php file we're replacing? Is it still used elsewhere?
Created attachment 425034 [details] Patch
(In reply to Jonathan Bedard from comment #42) > Comment on attachment 424987 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424987&action=review > > > LayoutTests/http/tests/misc/resources/check-query-param.py:21 > > +if 'HTTP_COOKIE' in os.environ: > > Might consider using get_cookies and request.update() > > > LayoutTests/http/tests/misc/resources/redirect-to-http-url.py:1 > > +#!/usr/bin/env python3 > > Where is the php file we're replacing? Is it still used elsewhere? See comment 41
Comment on attachment 425034 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425034&action=review > LayoutTests/http/tests/misc/resources/check-query-param.py:29 > + request.update({ key: cookies[key] }) Don't need a loop here, request.update(get_cookies()) should work
Created attachment 425036 [details] Patch
Committed r275561 (236216@main): <https://commits.webkit.org/236216@main>