Hi - > This patch aims to reduce the risk by adding an option to debuginfod > that functions kind of like an IP packet's TTL: a limit on the > length of XFF: header that debuginfod is willing to process. If > X-Forwarded-For: exceeds N hops, it will not delegate a local lookup > miss to upstream debuginfods. [...]
Thank you very much! > Commit ab38d167c40c99 causes federation loops for non-existent > resources to result in multiple temporary livelocks, each lasting > for $DEBUGINFOD_TIMEOUT seconds. [...] (FWIW, the term "livelock" is not quite right here, try just "deadlock".) The patch looks functional, and thank you also for including the docs and test case. Thorough enough! > @@ -1862,6 +1869,12 @@ handle_buildid (MHD_Connection* conn, > // We couldn't find it in the database. Last ditch effort > // is to defer to other debuginfo servers. > > + // if X-Forwarded-For: exceeds N hops, > + // do not delegate a local lookup miss to upstream debuginfods. > + if (disable_query_server) > + throw reportable_exception(MHD_HTTP_NOT_FOUND, "not found, > --forwared-ttl-limit reached \ > +and will not query the upstream servers"); One part I don't understand is why you added the code to check for XFF length into handler_cb(), and then passed the disable_query_server result flag to this function. Was there some reason not to perform the XFF comma-counting right here? - FChE
