Ticket #2416 (closed defect: duplicate)

Opened 9 months ago

Last modified 7 months ago

Spurious SAWarnings caused by built-in SQL-based auth

Reported by: pitrou Assigned to: Gustavo
Priority: normal Milestone: 2.1b2
Component: TurboGears Version: 2.1a1
Severity: normal Keywords:
Cc:

Description

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.

Change History

12/01/09 21:44:00 changed by jorge.vargas

  • milestone changed from __unclassified__ to 2.1b1.

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

12/11/09 02:35:25 changed by 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

(follow-up: ↓ 4 ) 01/29/10 04:34:40 changed by 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

(in reply to: ↑ 3 ) 01/29/10 18:28:40 changed by 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.

(follow-up: ↓ 6 ) 01/30/10 06:37:26 changed by 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.

(in reply to: ↑ 5 ) 01/31/10 17:57:34 changed by Gustavo

  • owner set to Gustavo.
  • status changed from new to assigned.

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.

(follow-ups: ↓ 8 ↓ 9 ) 02/01/10 11:19:17 changed by 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.

(in reply to: ↑ 7 ) 02/01/10 15:55:48 changed by 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).

(in reply to: ↑ 7 ) 02/01/10 16:05:13 changed by 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.

(follow-up: ↓ 11 ) 02/01/10 16:24:00 changed by 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.

(in reply to: ↑ 10 ) 02/01/10 16:59:19 changed by Gustavo

  • status changed from assigned to closed.
  • resolution set to duplicate.

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.