DSPAM broken after upgrade to PostgreSQL 9.1
Brought to you by:
paulcockings,
sbajic
Menu
▾
▴
#141 DSPAM broken after upgrade to PostgreSQL 9.1
After PostgreSQL 9.1 has hit Debian/testing DSPAM fails to insert data into the table 'dspam_signature_data' (see attached sql_erros.txt). After digging around in the source code I have seen the function 'PQescapeByteaConn' (pgsql_drv.c line 1574) is used to escape the data. Unfortunately with PostgreSQL 9.1 the default from the setting 'standard_conforming_strings' has changed from 'off' to 'on'. Please read the following thread as I am not an expert in C-Programming nor in PostgreSQL: http://postgresql.1045698.n5.nabble.com/PQescapeByteaConn-returns-wrong-string-for-PG9-1-Beta3-td4667713.html
I can confirm that DSPAM is working again if I set 'standard_conforming_strings = off' in my postgresql.conf. But since the new default for PostgreSQL 9.1 is 'standard_conforming_strings = on' this should be considered in pgsql_drv.c.
Hello Ingo, can you try latest GIT version of DSPAM and tell me if the problem still exists there?
Hi Stevan,
so far I have used the Debian packages from Julien Valroff (http://www.kirya.net/) which are available from the official Debian Backports repository. I will try to compile the pgsql part and the if problem still exists.
Regards,
Ingo
I got warnings with this fix and standard_conforming_strings to off and this resulted in DSPAM returning with a non-zero exit code and mail delivery failure. Maybe seeting escape_string_warning to off would help, as it warns if any backslash is found in the string, even if doubled.
As i can now switch standard_conforming_strings to on, i just did, then i should be able to ping this bug if the fix is not enough.
We still encounter this bug in Debian [0], even with dspam in version 3.10.2. Escape strings should be prefixed with a capital E when standard_conforming_strings is on (the default in PostgreSQL now) so I made a patch that does just that.
[0] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=719663
Cheers.
Last edit: Thomas Preud'homme 2013-11-26
Hello Thomas,
thank you for the patch. Are you sure it will fix your issues with strings? Inside the patch I see that you are prefixing the signature. Why? The signature it self is simple and does not have any backslash or so in it. IMHO the E in front the signature is not required. Does you patch work with standard_conforming_strings set to on and off too?
cheers,
Stevan
Coin,
Thomas's reasoning is right, see:
http://www.postgresql.org/docs/current/static/runtime-config-compatible.html
The E'' syntax was introduced in 8.1, and i don't think there's any
interest in supporting older version in DSPAM.
Quoting Stevan Bajic sbajic@users.sf.net:
For the signature there is no need for it if you don"t use backslashes
to escape, you're right.
If it is on, you must use E'' when you use backlashes to escape.
If it is off, you may use E'' but it will still work fine with a
warning (if not disabled with escape_string_warning set to false).
Regards.
--
Marc Dequènes (Duck)
Yes, exactly. The E prefix indicates to PostgreSQL that it is a string constant with escaping. In the absence of E prefix, it is a normal string constant. standard_conforming_strings only impact the way normal string constant are interpreted by PostgreSQL. When set to on, it will interpret every character literally, including the backslash. That is, the backslash is part of the string like any letter from a to z would be and does not introduce an escape sequence. When standard_conforming_strings is set to off, PostgreSQL will accept escape sequence in normal string constants. Prefixing with E means it will accept escape sequence in the next string constant, no matter what. If there is no escape sequence in a given string constant, then prefixing or not with E wouldn't change anything.
With regards to my patch, I didn't check the semantic of the strings constant, that is why I prefixed all of them with E. As explained above, it will work no matter what is the value of standard_conforming_strings. However if signature never contains characters that needs to be escaped, then the E prefix is useless (but not harmful).
I hope I answered your question.
Best regards,
Thomas
Oh and also in the code you already call function to escape signature so it was only consistent for me to add the E prefix. If you know these strings don't need to be escape, then you probably should remove the call PQescapeStringConn for these constant strings.
Hello Thomas,
the signature does not contain any special characters. So the E is not needed. Anyway.... if the E is not needed im the signature then I don't see how your patch is going to fix issues with escaping strings? The initial problem reported by Deboan folks still exists then. Right?
Stevan
TL;DR: not only signature are escaped + what is the content of SIG->data in line 1524 of pgsql_drv.c?
Great, that shorten the patch then. As you can see I changed all the string constant and not all of them seems to be about signature. For instance the 5th and 6th change in the patch are about a name (SELECT ... WHERE virtual_name = name_esc). If the name comes from the user it could contain some character that needs to escape. Take my last name for example (Preud'homme): the apostrophie that it contains needs to be escaped.
I'm less sure for the 7th and 8th changes in the patch as it is about the preferences. But if it's exactly what is written in the configuration file (I mean not filtered already), then it might contains any weird character as well.
As to the bug experienced in Debian, it is the second part of second change. It's in the case of SIG->data contains escape sequence. Indeed, it looks like a signature so there might be something weird. I'd appreciate if you can tell me what this data can contain. It might be a mistake I made as I just realized when doing a new upload that at some point I carried over a patch I made for dspam 3.10.1 in Debian to set standard_conforming_strings to off to dspam 3.10.2. Since a lot of code was added in 3.10.2 to escape strings, maybe a string was too escaped or something and that corrupted the database of a user. I'm not sure, I'd need to look more into it. Please let me know what SIG->data should contain and if it is abnormal that it contains an escape sequence.
Best regards,
Thomas
Hi Stevan,
after a new bug in Debian which I think is related to my latest change, I am trying to better understand what is going on. First of all, I am now sure that the bug we got in Debian [0] was indeed about a missing E prefix. If I understood correctly, SIG->data is the data associated to a signature and is a raw sequence of byte (bytea in postgresql terminology). Therefore, it needs to be escaped (as dspam currently does) and is very likely to contain escape sequence (by definition). Thus, it needs to be prefixed by E.
[0] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=719663
As to the signature, you indeed control how it looks like when you create it. But then, when you receive an email, you parse the header which contains the signature and you look if it exists in the database. Since it is an incoming email, it could be a forged signature that contains escape sequence. This could be made in an attempt to fill the /var/log partition of a dspam setup in order to create a DOS. So I believe except for the insertion of the signature and its associated data, all the other place where dspam manipulate a signature it might contains some escape sequence and should thus be prefixed with a E again.
I'm probably not teaching you anything since you correctly called PQescapeStringConn for the signature before doing any SQL request so you knew it could contains weird character. It is just the normal consequence to also prefix the escaped signature with a capital E, or postgresql could complain. I didn't investigate the other place where my patch adds a E prefix but it might be justified by following the same reasonning. Anyway, as explained before, adding the capital E doesn't hurt, unless you have a string that contains backslashes that you want to store as is.
I hope this email is clearer and more complete than the previous one.
Best regards,
Thomas
My bad, the E prefix is indeed not necessary. PQescapeStringConn and PQescapeByteaConn does less escaping when standard_conforming_strings is on so that the result of calling these escaping function can be passed as a normal string litteral (ie no need for E prefix which indicates escaped strings). We had the bug in Debian because a wrong merge pulled a patch to set standard_conforming_strings to off.
In other words, on our side (Debian) everything is fine and this bug can be closed (it is closed in Debian).