On 30/04/11 04:39, Alex Rousskov wrote:
On 04/29/2011 08:18 AM, Amos Jeffries wrote:
Are you aware of any convention about src,dst or dst,src pairing and
which goes first? All I can think of is the snprintf(out, out_max,
...) example.
I would mimic memcpy() and memmove(): dst, src.
However, std::copy() uses src,dst so you cannot go wrong either way :-)!
I'm tempted to alter the API to be (outBuf, outBufsz, inBuf, inBufSz)
before we get too many more callers for the new functions.
Good idea, IMO.
=== modified file 'src/HttpHeader.cc'
--- src/HttpHeader.cc 2011-03-22 12:23:25 +0000
+++ src/HttpHeader.cc 2011-04-29 13:13:00 +0000
@@ -1414,7 +1414,10 @@
if (!*field) /* no authorization cookie */
return NULL;
- return base64_decode(field);
+ static char decodedAuthToken[8192];
+ int decodedLen = base64_decode(decodedAuthToken, fields,
sizeof(decodedAuthToken));
+ decodedAuthToken[decodedLen] = '\0';
+ return decodedAuthToken;
}
s/fields/field/ ?
meh. yeah and a few other bits I've found tonight while build-testing.
Looking at this and other caller changes, it feels like you need
base64_decode_and_terminate() that will handle termination safely. If
you add that, please check that 64_decode_len() and/or its documentation
is adjusted accordingly.
This is the only decode caller which needs termination anymore. The
others just map the output to a binary packet structure. I'm pretty sure
this one can go as well with some fiddling, but that is not in scope.
It feels wrong that we ignore all decoding errors, but fixing that may
be outside your patch scope.
Which errors do you mean? the decoder skipping non-decodeable characters?
On the other end the callers of the src/HttpHeader.cc getAuth() method
handle incorrect output by validating the username:password string
decoded. In the NTLM helpers where the other decoding is done the
resulting decoded packet is validated for various binary properties
including decoded length.
=== modified file 'lib/base64.c'
+int
+base64_decode_len(const char *data)
+{
+ if (!data || !*data)
+ return 0;
+
+ int terminatorLen = 0;
+ int dataLen = strlen(data);
s/int dataLen/const int dataLen/
The !*data check is probably not needed.
BTW, I am not sure, but it is possible that base64_decode_len() can be
implemented more efficiently and/or more elegantly using strcspn(data,
"="), avoiding searching for \0 and then back-searching for \=. Not a
big deal though, even if strcspn() is indeed faster.
Good point. I think '=' is invalid within the encoded string. But not
completely sure. The current version will ignore it.
Fixed the other comments. Patch v3 pending build tests.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.12
Beta testers wanted for 3.2.0.7 and 3.1.12.1
