Spurious SAWarnings caused by built-in SQL-based auth

[pitrou]

SQLAlchemy-based authentication as setup by the quickstart fills the logs with such warning lines:

.../sqlalchemy/engine/default.py:230: SAWarning: Unicode type received non-unicode bind param value 'manager'
  param.append(processors[key](compiled_params[key]))

(note: the value between quotes is the user_name)

This is witnessed on 2.1a2 as well as 2.0.

[jorge.vargas]

I have seen this happen before. Somewhere r.what is using a vanilla str instead of unicode.

[danielfalk]

Further discussion on the list points to the problem as being with parse_formvars from paste.request.

 http://old.nabble.com/Unicode-handling-in-repoze-identification-td21993846.html

But it looks like still nobody has opened a ticket with paste yet

[chrisz]

There are actually two independent issues here. First, for ascii names you get these warnings. Second, non-ascii are currently not working at all, but this will be fixed in #2438.

To solve the first problem, we would need to convert ascii (str) values to unicode whenever we are using them in SQLAlchemy filter comparisons. As far as I see this happens at the following places:

• method _get_section_as_row in repoze.wha.plugins.sql.adapters
• method _get_item_as_row in repoze.wha.plugins.sql.adapters
• method get_user in repoze.who.plugins.sa

[Gustavo]

Replying to chrisz:

To solve the first problem, we would need to convert ascii (str) values to unicode whenever we are using them in SQLAlchemy filter comparisons. As far as I see this happens at the following places:

• method _get_section_as_row in repoze.wha.plugins.sql.adapters
• method _get_item_as_row in repoze.wha.plugins.sql.adapters
• method get_user in repoze.who.plugins.sa

Those methods should work with both ASCII and Unicode values, since they don't change or rely on a particular type of string.

I think this issue will be gone with #2438 too.

[chrisz]

Yes, the methods work with both str and unicode, but if you pass a str, then SA gives these warnings because you compare a Python str value with a unicode database colum. I don't think this will be solved with #2438, because request.POST will still return str values when there are no non-ascii chars. But we should finish #2438 first anyway, and then we can come back to this again.

[Gustavo]

Replying to chrisz:

Yes, the methods work with both str and unicode, but if you pass a str, then SA gives these warnings because you compare a Python str value with a unicode database colum. I don't think this will be solved with #2438, because request.POST will still return str values when there are no non-ascii chars. But we should finish #2438 first anyway, and then we can come back to this again.

According to my tests, the new repoze.who-friendlyform (which uses request.POST under the hood) is returning unicode for both ASCII and Unicode, that's why I think those warnings will be gone.

However, I think I'm going to make it return str when using ASCII just in case, but I think that's another story.

[chrisz]

Right, request.POST returns unicode when request.charset is set (which we now do).

I still think repoze.wha.plugins.sql.adapters and repoze.who.plugins.sa should convert str to unicode in the methods mentioned above when they get such input so SQLAlchemy will not give these warnings.

[Gustavo]

Replying to chrisz:

Right, request.POST returns unicode when request.charset is set (which we now do).

I still think repoze.wha.plugins.sql.adapters and repoze.who.plugins.sa should convert str to unicode in the methods mentioned above when they get such input so SQLAlchemy will not give these warnings.

I'll try to get rid of those warnings - But are you really getting them now that request.POST return the right string sub-type? I don't (with a fresh TG2 project).

[pitrou]

Replying to chrisz:

I still think repoze.wha.plugins.sql.adapters and repoze.who.plugins.sa should convert str to unicode in the methods mentioned above when they get such input so SQLAlchemy will not give these warnings.

My 2 cents: I think repoze should not try to do such a conversion. It doesn't know the charset, the caller (TurboGears) does, so it's up to the caller to do the conversion. Sure, repoze can use 'ascii' by default, but this will fool people into thinking that they're fine and one day they'll have a crash because of non-ASCII user data.

[chrisz]

Gustavo: No, I haven't seen these warnings any more after the latest changes.

Pitrou: Ok, I can follow you on that.

So we should probably just close this ticket and reopen/rediscuss in the case that these warnings pop up again.

[Gustavo]

Replying to chrisz:

Gustavo: No, I haven't seen these warnings any more after the latest changes.

Pitrou: Ok, I can follow you on that.

So we should probably just close this ticket and reopen/rediscuss in the case that these warnings pop up again.

Cool! ;-)

Closing it as a duplicate of #2438.