<div dir="auto">I really like the making safe of the error log as opposed to truncation. The more information logged in cases like this the better.<div dir="auto"><br></div><div dir="auto">Alternatively what about something that indicates further data was truncated?</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 28 Sep 2022, 21:07 Dipl. Ing. Sergey Brester via nginx-devel, <<a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><u></u>
<div style="font-size:10pt;font-family:Verdana,Geneva,sans-serif">
<p>Sure, this was also my first intention. Just after all I thought the whole buffer <br>could be better in order to provide a possibility to debug for someone searching <br>for a bug. But there are another aids that would help, so indeed let it be so.<br><br>As for the rest, well it is surely a subject of different discussion. However I think <br>someone who monitoring logs with foreign data is able to ignore wrong chars<br>or unsupported surrogates using appropriate encoding facilities, but...<br>But I would suggest at least to replace a quote-char to provide opportunity<br>to bypass the buffer in quotes with something similar this `... header: "[^"]*" ...`<br>if parsing of such lines is needed.</p>
<p>So may be this one either:</p>
<pre style="font-family:courier new,courier">+    for (p = buf; p != last; p++) {
<span style="color:#0000ff"><strong>+        if (*p == '"') {
+            *p = '\''; continue;
+        }
</strong></span>+        if (*p == CR || *p == LF) {
+            break;
+        }
+    }
</pre>
<p>Although I don't believe the safe data (like IP, etc) shall take place after "unsafe" <br>(foreign) input (especially of variable length), but it is rather a matter of common <br>logging format for the error-log.<br>I mean normally one would rather expect something like that:</p>
<pre><span style="font-family:courier new,courier;color:#ff0000"><span style="font-family:courier new,courier">- [error] 13868#6132: *1 broken header: "...unsafe..." <span style="font-family:courier new,courier"><span style="font-family:courier new,courier">while reading PROXY protocol, client: 127.0.0.1, server: <a href="http://0.0.0.0:80" target="_blank" rel="noreferrer">0.0.0.0:80</a></span></span></span></span></pre>
<pre><span style="font-family:courier new,courier;color:#008000">+ [error] 13868#6132: while reading PROXY protocol, client: 127.0.0.1, server: <a href="http://0.0.0.0:80" target="_blank" rel="noreferrer">0.0.0.0:80</a> - *1 broken header: "...unsafe..."</span></pre>
<p>Unfortunatelly errorlog is not configurable at the moment at all.</p>
<p>28.09.2022 12:02, Roman Arutyunyan wrote:</p>
<blockquote type="cite" style="padding-left:5px;border-left:#1010ff 2px solid;margin-left:5px">
<pre>Hi Sergey,

Thanks for reporting this.  The issue indeed needs to be fixed.  Attached is 
a patch similar to yours that does this.  I don't think we need to do anything
beyond just cutting the first line since there's another similar place in
nginx - ngx_http_log_error_handler(), where exactly that is implemented.

Whether we need to skip special characters when logging to nginx log is
a topic for a bigger discussion and this will require a much bigger patch.
I suggest that we only limit user data to the first line now.

[..]

--
Roman Arutyunyan
</pre>
</blockquote>
</div>
_______________________________________________<br>
nginx-devel mailing list -- <a href="mailto:nginx-devel@nginx.org" target="_blank" rel="noreferrer">nginx-devel@nginx.org</a><br>
To unsubscribe send an email to <a href="mailto:nginx-devel-leave@nginx.org" target="_blank" rel="noreferrer">nginx-devel-leave@nginx.org</a><br>
</blockquote></div>