367214 - mod_cern_meta support in the HTTP server

Status: RESOLVED FIXED

(Testing :: General, enhancement)

Description

[Jeff Walden [:Waldo]]

There exist numerous cases when it is necessary to modify the headers sent with the file sent in response to an HTTP request.  The Apache module mod_cern_meta stores header data in a separate file in a sibling directory, e.g. for the file /foo/bar.html headers would be in /foo/.web/bar.html.meta.

Fixing this bug is next on my to-do list for the server after completing POST support (bug 366371), which is done and awaiting review.  Depending on my mood when that's finished it's possible I might also fix bug 366932 before this, but at the moment this bug interests me more than that bug.  ;-)

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

For any file explicitly registered or contained within a directory mapped to a virtual server path, the file ("_headers_" + leaf name) contains extra HTTP header/status line data to add to the response.  The format is as follows, with some definitions imported from RFC 2616:

HeaderFile  =  [ Status-Line ] *Header-Line
LINEEND     = CR | LF | CRLF | LFCR | EOF
Status-Line = "HTTP" SP Status-Code SP Reason-Phrase LINEEND
Header-Line = field-name ":" [ field-content ] LINEEND

Getting a non-colliding name for the metadata file required tweaking file storage a little.  Now, any file whose name doesn't start with an underscore is represented by the same file.  Any file whose name begins with an underscore is represented by the file with an underscore prepended to the name.  Consequently, any file whose name starts with exactly one underscore is a server-specific file and will not be served by the server, displayed in directory listings, etc.

A significant portion of this patch is tests, so it's not really as big as it seems.  There's also a little refactoring to use Components.Constructor which shouldn't be difficult to ignore.  There's also some refactoring to do in the index-handler test which I'm going to push to a followup bug.

This is something reftest has wanted for awhile (and once reftest uses this, it'd fix the occasional orange on the WinXP box due to the test that requires a 404 error not working correctly), so it'd be good to get this sooner rather than later.

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Some tweaks

I didn't realize I'd left an XXX comment in the code related to when path decoding occurred.  This patch does a slight refactoring to remove it, and it makes a few readability tweaks at the same time.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 262232 [details] [diff] [review]
Some tweaks

  See the docs at
+ * http://developer.mozilla.org/en/docs/Components.Constructor for details.

OK, I'm not sure that it's necessary to explain that here, but whatever...

+    var valid = lis.readLine(line);

That's not what the return value means... see the IDL :)

+        var code = status;
+        var description = "";
+      }
+      else
+      {
+        code = status.substring(0, space);
+        description = status.substring(space + 1, status.length);

OK, technically this is correct JavaScript, but can you do people who are used
to more traditional languages a favor and declare code and description before
this if?

+  /** Stream listener for the channels. */

looks alot like an nsIStreamLoader... except that you also get an
onStartRequest I guess


I'm not really a fan of using DOMParser in a necko test :/

Comment 4

[Jeff Walden [:Waldo]]

Attachment: As checked in

(In reply to comment #3)
> That's not what the return value means... see the IDL :)

Point, changed the name to more.


> OK, technically this is correct JavaScript, but can you do people who are used
> to more traditional languages a favor and declare code and description before
> this if?

Grudgingly, yes.


> I'm not really a fan of using DOMParser in a necko test :/

Seems more principled than anything else, and you're going to be testing a DOM anyway; I don't see any other way to do it.


Aside from those, the other changes here consist of:

- changing the hiding scheme: instead of _ at beginning, it's now ^ at end,
  mostly because a leading _ isn't entirely uncommon (we even have one in
  Mochitest, but I don't think we use it); also means a file and its headers
  appear next to each other in an alphabetical sort
  - do a bunch of drudge renaming of files and functions to reflect the
    underscore -> caret switch
- fix an issue with URL-decoding the title of an index page
- fix an issue in test code for the index handler with the encoding of hrefs
  for children of a directory

For the most part these changes were fairly menial, so I didn't bother re-requesting review.

I'll add documentation of this new header-setting capability to the relevant docs and newsgroups sometime later this week.

Comment 5

[Jeff Walden [:Waldo]]

Fixed.

Comment 6

[Jeff Walden [:Waldo]]

Moving httpd.js bugs to the new Testing :: httpd.js component; filter out this bugmail by searching for "ICanHasHttpd.jsComponent".

Comment 7

[Jeff Walden [:Waldo]]

...and changing the QA contact as well.  Filter on the string "BugzillaQAContactHandlingIsStupid".