Summary: | pywebsocket should support html and cgi in the same directory. | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fumitoshi Ukai <ukai> | ||||||||||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | ap, cjerdonek, eric, levin | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | All | ||||||||||||||||||
OS: | All | ||||||||||||||||||
Bug Depends on: | 34871 | ||||||||||||||||||
Bug Blocks: | |||||||||||||||||||
Attachments: |
|
Description
Fumitoshi Ukai
2010-02-12 00:06:54 PST
http://code.google.com/p/pywebsocket/issues/detail?id=43 in upstream issue tracker. Created attachment 48879 [details]
Patch
+ specify --server_host 127.0.0.1 -x /websocket/tests
What is --server-host, and why is it good to specify it? I think I know the answer, but please explain it in ChangeLog.
Seems untidy to have a mix of short and long option names.
> if '..' in self.path:
> return False
Do functions that we use for path manipulation perform tilde expansion (for home directory)? Writing a truly secure server is out of scope for the test tool, it's just too similar to existing check for "..", so I wanted to ask.
+ scriptfile = self.translate_path(self.path.split('?', 2)[0])
I don't understand what this line does. Someone who is better familiar with Python might understand it, and give their r+.
(In reply to comment #3) > + specify --server_host 127.0.0.1 -x /websocket/tests > > What is --server-host, and why is it good to specify it? I think I know the > answer, but please explain it in ChangeLog. Ok. > Seems untidy to have a mix of short and long option names. Changed to use long option names. > > if '..' in self.path: > > return False > > Do functions that we use for path manipulation perform tilde expansion (for > home directory)? Writing a truly secure server is out of scope for the test > tool, it's just too similar to existing check for "..", so I wanted to ask. I believe it doesn't perform tilde expansion. > + scriptfile = self.translate_path(self.path.split('?', 2)[0]) > > I don't understand what this line does. Someone who is better familiar with > Python might understand it, and give their r+. Created attachment 48963 [details]
Patch
Comment on attachment 48963 [details] Patch > + "--server_host", "127.0.0.1", > + "--port", "$webSocketPort", > + "--document_root", "$webSocketHandlerDir", > + "--scan_dir", "$webSocketHandlerScanDir", > + "--websock_handlers_map_file", "$webSocketHandlerMapFile", > + "--cgi_paths", "/websocket/tests", > + "--log_file", "$logFile", I think that the usual Unix style for long options is to use dashes, not underscores. (In reply to comment #6) > (From update of attachment 48963 [details]) > > + "--server_host", "127.0.0.1", > > + "--port", "$webSocketPort", > > + "--document_root", "$webSocketHandlerDir", > > + "--scan_dir", "$webSocketHandlerScanDir", > > + "--websock_handlers_map_file", "$webSocketHandlerMapFile", > > + "--cgi_paths", "/websocket/tests", > > + "--log_file", "$logFile", > > I think that the usual Unix style for long options is to use dashes, not > underscores. We've used userscores... Since this options is in pywebsocket, and no WebKitTools/Scripts uses underscores, should I fix it to use dashes? Comment on attachment 48963 [details]
Patch
Underscores are a strange Google convention. WebKit-land uses dashes. Also, you patch does not appear to apply cleanly to top of tree.
(In reply to comment #8) > (From update of attachment 48963 [details]) > Underscores are a strange Google convention. WebKit-land uses dashes. I think pywebsocket is third party code, so don't need to flow WebKit-style. > Also, > you patch does not appear to apply cleanly to top of tree. Hmm, this is webkit-patch (with git) bug, when moving files and directory becomes empty? webkit-patch apply-attachment says $ ./WebKitTools/Scripts/webkit-patch apply-attachment 48963 [details] Cleaning working directory Updating working directory Reading Keychain for bugs.webkit.org account and password. Click "Allow" to continue... Logging in as ukai@chromium.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=48963&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=34879&ctype=xml Processing 1 patch from 1 bug. Processing patch 48963 from bug 34879. Failed to run "['/Users/ukai/src/chromium-webkit2/src/third_party/WebKit/WebKitTools/Scripts/svn-apply']" exit_code: 2 Last 500 characters of output: h fuzz 3. patching file WebKitTools/Scripts/run-webkit-tests patching file WebKitTools/Scripts/run-webkit-websocketserver patching file WebKitTools/Scripts/webkitpy/layout_tests/port/websocket_server.py patching file WebKitTools/pywebsocket/mod_pywebsocket/standalone.py patching file WebKitTools/pywebsocket/setup.py Could not open 'LayoutTests/websocket/tests/cookies' to list files: 0 at /Users/ukai/src/chromium-webkit2/src/third_party/WebKit/WebKitTools/Scripts/svn-apply line 314, <> line 417. Failed to run "['/Users/ukai/src/chromium-webkit2/src/third_party/WebKit/WebKitTools/Scripts/svn-apply']" exit_code: 2 ukai-macpro:WebKit ukai$ git reset --hard Created attachment 48992 [details]
Patch
Created attachment 49188 [details]
Patch
FYI, we now have a README.webkit file in pywebsocket's directory that can be updated whenever local changes are made to pywebsocket: https://bugs.webkit.org/show_bug.cgi?id=35198 Created attachment 49714 [details]
Patch
Created attachment 49885 [details]
Patch
I think I could review this patch if you explained what this line did: + scriptfile = self.translate_path(self.path.split('?', 2)[0]) It seems worth both a short comment, and a longer explanation in the bug. Created attachment 49969 [details]
Patch
(In reply to comment #15) > I think I could review this patch if you explained what this line did: > > + scriptfile = self.translate_path(self.path.split('?', 2)[0]) it gets real pathname from request path. self.path contains request path in HTTP request header self.path.split('?', 2)[0] strips query parameter if any self.translate_path() converts resource name into pathname on filesystem. > It seems worth both a short comment, and a longer explanation in the bug. added comments I'm not sure what the svn-apply bug which this is tripping over is: Could not open 'LayoutTests/websocket/tests/cookies' to list files: 0 at /mnt/git/webkit-chromium-ews/WebKitTools/Scripts/svn-apply line 314, <> line 715. (In reply to comment #18) > I'm not sure what the svn-apply bug which this is tripping over is: > Could not open 'LayoutTests/websocket/tests/cookies' to list files: 0 at > /mnt/git/webkit-chromium-ews/WebKitTools/Scripts/svn-apply line 314, <> line > 715. Hmm, this is because the patch is trying to remove the directory? Comment on attachment 49969 [details]
Patch
r=me. Please land this with svn - as far as I know, git wrappers won't preserve history of file copies correctly.
Committed r55719: <http://trac.webkit.org/changeset/55719> |