Hi Jeremy, Committed a modified version of the last patch. Here I'm adding some comments. On Thu, Dec 04, 2008 at 11:11:03AM +0000, Jeremy Henty wrote:
OK, here it is. Please comment. Some observations:
When the cache parses the HTTP header we just attach any authentication headers to the cache entry. Nothing else happens until we process the clients. Just before iterating over the clients we check if all the data is in. If it is then we do any necessary authentication.
Just moved the test after the for loop.
Opening the authentication dialog runs the FLTK event loop, which means that incoming data can call the cache again, triggering CCC warnings. We work around it by deferring the dialog to a timeout so that the cache finishes iterating over the clients before the dialog appears. This is annoying but I don't see a better way at the moment.
That's the way to do it.
The authentication dialog does not know which browser window opened the URL so it can't call a_Nav_reload(). Instead I implemented a_Nav_reload_url().
Removed a_Nav_reload_url() and set bw as a parameter.
There's an interesting case when the server demands authentication for a realm that we have already authenticated ourselves to. It could be that we didn't send authentication because we didn't know the path was in the realm, or it could be that the authentication we sent was wrong. This requires the code to deduce what it did last time it saw this URL and rely on what it will do the next time. It works, but it feels fragile to me. I would prefer the URL to explicitly record authentication attempts so that the code doesn't have to second-guess itself. Thoughts?
This will need debugging. Doesn't seem critical though.
I implemented a full token-value list parser as per the RFC. Maybe this is overkill for Basic authentication, but it will be necessary for Digest authentication. Yes, it has gotos; I began by setting flags but decided that gotos were actually cleaner. Maybe it should be rewritten in state-machine style?
I'd appreciate it in smaller functions and without gotos.
If the user opens several pages in a realm in a very short time then it is conceivable that multiple authentication dialogs for the same realm could pop up (just like the https DPI sometimes does with certificate dialogs). I don't address this at the moment because I think it won't be a problem in practice. If it needs fixing then we will have to monitor the list of currently open dialogs to avoid duplicates.
Just added a check. It doesn't solve the problem, but avoids the storm and sends a warning to stdout. Please review the changes. It almost has no testing. Some changes I remember: - Modified a bit the code ignoring the last '/' of a path. - Reordered function so no massive forward declarations are necessary. - Used VOID2INT macro. - Added some comments (there're still plenty of uncommented functions) - Prefixed all functions in auth.c with "Auth" - Suffixed "_t" the typedefed data structures. - Removed a_Bw_each() - Removed compiler warnings. - some others... * Does Auth_path_is_inside() need to be case sensitive? Well, now it seems is time to fix the missing points and to real-world test it! -- Cheers Jorge.-