Re: lynx-dev [patch] 2.8.3dev.16 - cookie_save_file

[brian j pardy]

On Boomtime, the 60th of The Aftermath, 3165, Klaus Weide wrote:
> On Wed, 15 Dec 1999, brian j pardy wrote:
> 
> > What do you think of this?
> 
> Problem with this:
> 
> >                       } else {
> > +                        if (dump_output_immediately)
> > +                            return(0);
> > +
> >                          exit_immediately_with_error_message(NULLFILE, 
> > first_file);
> >                          return(-1);
> 
> You are short-circuiting here the exit_immediately_with_error_message
> that *should* happen for some cases.  As a result, Lynx will exit
> in some cases, with -dump and -source, returning 0 and no message
> instead of -1 and a message.
> 
> To limit this "damage", you could check HTOutputFormat in addition to
> dump_output_immediately.  If it is HTAtom_for("www/dump") or HTAtom_for(
> "www/download"), you probably know that it's a -source or a -mime_header
> but not just a -dump.
> 
> But there would still be some errors with -source (or -mime_header) that
> are returned by getfile() as NULLFILE and should be recognized as
> an error by mainloop() and produce a message.  But unfortunately we can't
> tell those error cases apart from a "successful" NULLFILE.

I figured there was something like that.  I did some really quick
testing with -source and -dump on obviously invalid URLs and fake
files, and it worked (exited with an error), but I definitely didn't
get any obscure errors tested.

> > Change HTFWriter_free() so that it is allowed to return instead of
> > calling exit_immediately().  This will (somehow -- I traced it from
> > the back, not the front) lead to getfile() returning NULLFILE.
> 
> Question is: why?

I left it as NULLFILE because I assumed that since it (-source) was
returning that presently, that NULLFILE was the "proper" return value.

> I assume HTLoadDocument() is receiving HT_LOADED from HTLoad(), and returning
> YES to getfile().  (Check -trace for "HTAccess: ..." entries, compare with
> end of HTLoadDocument.)
> 
> So it must me getfile() that returns NULLFILE, in spite of the YES.  Why
> the heck would it do so?  Well, because of these checks:
> 
>                         } else if (pound == NULL &&
>                                    /* ...
>                                     */
>                                    (strcmp(doc->address,
>                                            HTLoadedDocumentURL()) ||
>                                     ...
> 
> There is no HTMainText at all, so there appears to be no match between
> requested document (doc) and loaded document (HTMainText).  Btw. the
> relevant comment for this has become separated from the code, it is
> further above:
>                     /*
>                      *  Some URL's don't actually return a document;
>                      *  compare doc->address with the document that is
>                      *  actually loaded and return NULLFILE if not
>                      *  loaded.  If www_search_result is not -1
>                      *  then this is a reference to a named anchor
>                      *  within the same document; do NOT return
>                      *  NULLFILE in that case.
>                      */
>
> Now this suggests an experiment: wat happens if you append a '#' or
> '#something' to your URL?  Does it still work?  I assume not (but
> haven't tested *any* of this.)

-source with a URL trailed with '#something' segfaults.  Oops.

> So maybe getfile() should be tweaked, to always return NORMAL in a
> case that really *is* a success with -source, and not check HTMainText
> at all in that case (or check that it doesn't exist...)

I considered that at first, and then somehow convinced myself not to.
I can't remember why now.  I was probably just lazy and didn't want to
parse through all the checks that determine what to return.

I'll see if I can come up with anything in getfile() to figure out if
it was indeed a successful -source (etc) operation.

> > The
> > LYMainLoop.c change there is for when we get a NULLFILE back from
> > getfile()
> 
> And the successful return from -source retrieval would then fall under
> case NORMAL:.  Possible no added code is needed - the following
> return(0) might already catch it:
> 
>         } /* end if (LYforce_no_cache || force_load || are_different(...)) */
> 
>         if (dump_output_immediately) {
>             if (crawl) {
>                 print_crawl_to_fd(stdout, curdoc.address, curdoc.title);
>             } else {
>                 print_wwwfile_to_fd(stdout,0);
>             }
>             return(0);
> 
> (print_wwwfile_to_fd() won't do anything, without a HTMainText.  You're
> already relying on similar behavior from printlist() btw.)

AFAICT that return would indeed handle it.

> Seems complicated?  He, now you know why I didn't want to touch it...

:)  It's an excuse for me to get at least slightly more into the
internals, everything I've done so far has been pretty much limited to
LYCookie.c and supporting files.

-- 
Hearts will never be practical until they can be made unbreakable.
                -- The Wizard of Oz