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

[Klaus Weide]

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.

> 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 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.)

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...)

> 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.)

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

> -- if we're in dump mode, go ahead and return a 0 from
> mainloop().  Since we're calling this from dump mode, all that happens
> afterward is to print the references list (if necessary), then to do
> LYMain.c's already existing LYStoreCookies (if persistent &&
> EXP_PERSISTENT_COOKIES), then call cleanup_files() and
> exit_immediately(status) (0).
> 
> Since calling cleanup_files() seems to be safe for -dump, at the
> least, I just assumed it would be safe for a -source as well.  I don't
> know what it might be free'ing that isn't allocated in -source mode,
> so I wouldn't be surprised if someone else got a segfault on this.  I
> haven't had any.  I don't understand enough (read: anything) to know
> if various newdoc etc pointers will have been populated and improperly
> free'd.

I don't think there are any problems in this regard.


   Klaus