rpc: Make HTTP RPC debug logging more informative #14618

practicalswift · 2018-10-31T10:45:31Z

Make HTTP RPC debug logging more informative
Avoid excessively large log messages (which could theoretically fill up the disk) when running with debug option -debug=http

laanwj · 2018-10-31T16:47:32Z

utACK a6cf746
thanks!

kristapsk · 2018-10-31T17:01:10Z

utACK a6cf746b43686c1c2307b31308d2d6cd18ddb696

luke-jr · 2018-10-31T18:00:44Z

utACK

hebasto · 2018-10-31T21:28:20Z

utACK a6cf746b43686c1c2307b31308d2d6cd18ddb696

ken2812221

Concept ACK

src/httpserver.cpp

jgarzik · 2018-11-01T14:47:55Z

utACK

However, it must be noted that the HTTP server has always lacked proper request logging.

Having a separate log stream with logs parse-able by HTTP log analysis tools would be nice to have.

practicalswift · 2018-11-01T16:04:59Z

@ken2812221 Thanks for reviewing. Now allowing all characters allowed in URIs.

Please re-review :-)

luke-jr · 2018-11-02T05:29:40Z

Related (to adding new chars for SanitizeString): #10731

DrahtBot · 2018-11-02T14:43:08Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

promag · 2018-11-02T14:51:12Z

@jgarzik see discussion in #13992.

promag

utACK b301fecb.

promag · 2018-11-03T15:27:38Z

src/httpserver.cpp

        hreq->WriteReply(HTTP_BADMETHOD);
        return;
    }

+    LogPrint(BCLog::HTTP, "Received a %s request for %s from %s\n",
+             RequestMethodString(hreq->GetRequestMethod()), SanitizeString(hreq->GetURI(), SAFE_CHARS_URI).substr(0, 100), hreq->GetPeer().ToString());


What's the deal with 100 chars?

it's nothing special simply a sanity limit that avoids logging huge entire requests

sipa · 2018-11-03T19:17:59Z

utACK b301fecb830ad8ca9ec2419978ae8e0b6fbc6754

jonasschnelli · 2018-11-04T13:33:12Z

utACK b301fecb830ad8ca9ec2419978ae8e0b6fbc6754

DrahtBot · 2018-11-05T11:16:33Z

Needs rebase

practicalswift · 2018-11-05T12:27:24Z

Rebased! :-)

ab8c6f2 Add SAFE_CHARS[SAFE_CHARS_URI]: Chars allowed in URIs (RFC 3986) (practicalswift) 9912486 rpc: Make HTTP RPC debug logging more informative (practicalswift) Pull request description: * Make HTTP RPC debug logging more informative * Avoid excessively large log messages (which could theoretically fill up the disk) when running with debug option `-debug=http` Tree-SHA512: 9068862fb7d34db1e12e6b9dde78b669b86c65b4fed3ea8c9eb6c35310d77fd12b16644728fd7e9fbf25059d25114bded9e061eb3de649d8847486ec42041ce9

Github-Pull: bitcoin#14618 Rebased-From: 9912486

Github-Pull: bitcoin#14618 Rebased-From: ab8c6f2

fanquake · 2018-11-29T10:44:32Z

Backported in #14835.

f9db08e qa: Avoid race in p2p_invalid_block by waiting for the block request (MarcoFalke) 7935881 Add SAFE_CHARS[SAFE_CHARS_URI]: Chars allowed in URIs (RFC 3986) (practicalswift) 9666dba rpc: Make HTTP RPC debug logging more informative (practicalswift) b901578 add test demonstrating addrLocal UB (Kaz Wesley) 6f04264 fix uninitialized read when stringifying an addrLocal (Kaz Wesley) 5782fdc Throw error if CPubKey is invalid during PSBT keypath serialization (Gregory Sanders) de5e48a Bugfix: RPC: Add address_type named param for createmultisig (Luke Dashjr) df5131b gui: explicitly disable "Dark Mode" appearance on macOS (fanquake) Pull request description: Backports #14593, #14596, #14618, #14690 and #14728 to the 0.17 branch. Tree-SHA512: fcda4b75fcb71bb80cc8bde2a2b98ff5c0239dfa754ac980b1a91a90409502ac7678326399a4fc03a773074339dbf8b3d11750c91fe4302741a954745acfcca1

ab8c6f2 Add SAFE_CHARS[SAFE_CHARS_URI]: Chars allowed in URIs (RFC 3986) (practicalswift) 9912486 rpc: Make HTTP RPC debug logging more informative (practicalswift) Pull request description: * Make HTTP RPC debug logging more informative * Avoid excessively large log messages (which could theoretically fill up the disk) when running with debug option `-debug=http` Tree-SHA512: 9068862fb7d34db1e12e6b9dde78b669b86c65b4fed3ea8c9eb6c35310d77fd12b16644728fd7e9fbf25059d25114bded9e061eb3de649d8847486ec42041ce9

Summary: ab8c6f24d28ea1d1e6258cf316b4b97a0baf2377 Add SAFE_CHARS[SAFE_CHARS_URI]: Chars allowed in URIs (RFC 3986) (practicalswift) 991248649b76a5a071e1360a700f3e2ecf3e1e1f rpc: Make HTTP RPC debug logging more informative (practicalswift) Pull request description: * Make HTTP RPC debug logging more informative * Avoid excessively large log messages (which could theoretically fill up the disk) when running with debug option `-debug=http` Tree-SHA512: 9068862fb7d34db1e12e6b9dde78b669b86c65b4fed3ea8c9eb6c35310d77fd12b16644728fd7e9fbf25059d25114bded9e061eb3de649d8847486ec42041ce9 --- This is a backport of Core [[bitcoin/bitcoin#14618 | PR14618]] Test Plan: ninja check Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5747

ab8c6f2 Add SAFE_CHARS[SAFE_CHARS_URI]: Chars allowed in URIs (RFC 3986) (practicalswift) 9912486 rpc: Make HTTP RPC debug logging more informative (practicalswift) Pull request description: * Make HTTP RPC debug logging more informative * Avoid excessively large log messages (which could theoretically fill up the disk) when running with debug option `-debug=http` Tree-SHA512: 9068862fb7d34db1e12e6b9dde78b669b86c65b4fed3ea8c9eb6c35310d77fd12b16644728fd7e9fbf25059d25114bded9e061eb3de649d8847486ec42041ce9

fanquake added the RPC/REST/ZMQ label Oct 31, 2018

ken2812221 reviewed Nov 1, 2018

View reviewed changes

src/httpserver.cpp Outdated Show resolved Hide resolved

practicalswift force-pushed the limit-rpc-request-logging branch from d5505c3 to b301fec Compare November 1, 2018 16:08

promag reviewed Nov 3, 2018

View reviewed changes

DrahtBot added the Needs rebase label Nov 5, 2018

practicalswift added 2 commits November 5, 2018 13:27

rpc: Make HTTP RPC debug logging more informative

9912486

Add SAFE_CHARS[SAFE_CHARS_URI]: Chars allowed in URIs (RFC 3986)

ab8c6f2

practicalswift force-pushed the limit-rpc-request-logging branch from b301fec to ab8c6f2 Compare November 5, 2018 12:27

laanwj merged commit ab8c6f2 into bitcoin:master Nov 5, 2018

fanquake added Needs backport and removed Needs rebase labels Nov 29, 2018

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Nov 29, 2018

rpc: Make HTTP RPC debug logging more informative

9666dba

Github-Pull: bitcoin#14618 Rebased-From: 9912486

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Nov 29, 2018

Add SAFE_CHARS[SAFE_CHARS_URI]: Chars allowed in URIs (RFC 3986)

7935881

Github-Pull: bitcoin#14618 Rebased-From: ab8c6f2

fanquake mentioned this pull request Nov 29, 2018

[0.17] Further Backports #14835

Merged

fanquake removed the Needs backport label Nov 29, 2018

practicalswift deleted the limit-rpc-request-logging branch April 10, 2021 19:36

DeckerSU mentioned this pull request Jul 12, 2021

Patch CVE 2018 20586 pbcllc/sfusd-core#9

Merged

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

rpc: Make HTTP RPC debug logging more informative #14618

rpc: Make HTTP RPC debug logging more informative #14618

Uh oh!

Conversation

practicalswift commented Oct 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Oct 31, 2018

Uh oh!

kristapsk commented Oct 31, 2018

Uh oh!

luke-jr commented Oct 31, 2018

Uh oh!

hebasto commented Oct 31, 2018

Uh oh!

ken2812221 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jgarzik commented Nov 1, 2018

Uh oh!

practicalswift commented Nov 1, 2018

Uh oh!

luke-jr commented Nov 2, 2018

Uh oh!

DrahtBot commented Nov 2, 2018

Conflicts

Uh oh!

promag commented Nov 2, 2018

Uh oh!

promag left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

promag Nov 3, 2018

Choose a reason for hiding this comment

Uh oh!

laanwj Nov 5, 2018

Choose a reason for hiding this comment

Uh oh!

sipa commented Nov 3, 2018

Uh oh!

jonasschnelli commented Nov 4, 2018

Uh oh!

DrahtBot commented Nov 5, 2018

Uh oh!

practicalswift commented Nov 5, 2018

Uh oh!

fanquake commented Nov 29, 2018

Uh oh!

Uh oh!

practicalswift commented Oct 31, 2018 •

edited

Loading

promag left a comment •

edited

Loading