[PATCHv2] debuginfod: Check result of calling MHD_add_response_header.

[Mark Wielaard]

Hi Frank,

On Wed, 2021-12-01 at 10:23 -0500, Frank Ch. Eigler wrote:
> > Although unlikely the MHD_add_response_header can fail for
> > various reasons.  If it fails something odd is going on.
> > So check we can actually add a response header and log an
> > error if we cannot.
> 
> TBH I wouldn't bother even this much checking.  It just uglifies the
> code.  If it would make covscan happier, I'd stick a (void) in front
> of the add-header calls.

That is really just like ignoring the issue imho. But you are right
that it uglifies the code, I'll wrap the calls in an helper function.

> > -    MHD_add_response_header (r, "Content-Type", "text/plain");
> > -    MHD_RESULT rc = MHD_queue_response (c, code, r);
> > +    MHD_RESULT rc1, rc2;
> > +    rc1 = MHD_add_response_header (r, "Content-Type",
> > "text/plain");
> > +    rc2 = MHD_queue_response (c, code, r);
> >      MHD_destroy_response (r);
> > -    return rc;
> > +    return (rc1 == MHD_NO || rc2 == MHD_NO) ? MHD_NO : MHD_YES;
> 
> e.g. this part won't work: returning MHD_NO causes libmicrohttpd to
> send a 503 error back to the caller, regardless of our intended one.

OK, so we only want to report MHD_NO here when MHD_queue_response
fails.

> > +        if (MHD_add_response_header (resp, "Last-Modified",
> > datebuf) == MHD_NO)
> > +          if (verbose)
> > +            obatched(clog) << "Error: couldn't add Last-Modified
> > header"
> > +                           << endl;
> >      }
> 
> e.g., we normally report errors to the logs, regardless of verbosity
> settings.

OK, I'll remove the if (verbose).

> > +      if (MHD_add_response_header (r, "Content-Type",
> > +                                   "application/octet-stream") ==
> > MHD_NO
> > +          || MHD_add_response_header (r, "X-DEBUGINFOD-SIZE",
> > +                                      to_string(s.st_size).c_str()
> > ) == MHD_NO
> > +          || MHD_add_response_header (r, "X-DEBUGINFOD-FILE",
> > +                                      file.c_str()) == MHD_NO)
> 
> e.g., this formulation makes it impossible to add some headers if a
> previous one failed.

It is likely that if one fails, then all others fail similarly, but I
see your point. Any header is better than no headers at all.

Thanks,

Mark