Re: Review Board Ticket #4988: Fail to post diff with perforce repo: 'Value to convert is unexpected type %s', <class 'rev iewboard.scmtools.core.Revision'>

2 views
Skip to first unread message

David Trowbridge

unread,
Feb 6, 2023, 4:39:41 PM2/6/23
to David Trowbridge, Ben Jackson, reviewboa...@googlegroups.com
To reply, visit https://hellosplat.com/s/beanbag/tickets/4988/

New update by david

For Beanbag, Inc. Review Board Ticket #4988

Fixed in release-4.0.x (99b1e7e).

This will ship in the upcoming 5.0.2 and (at a later point), 4.0.12

Christian Hammond

unread,
Feb 6, 2023, 10:23:01 PM2/6/23
to Christian Hammond, Ben Jackson, reviewboa...@googlegroups.com

New update by chipx86

Found a regression with the fix, but we'll get reworked version in for 5.0.2 (which is going out tonight/tomorrow).

force_text()/force_str() will assume an encoding if the object is a bytes or converts to a bytes. The problem with this is that we can't assume at this particular stage. We instead need to use the configured encoding list. This matters particularly for people working with some Chinese encodings. convert_to_unicode() is responsible for trying the configured encodings, but since the fix was passing in the forced-encode, it never got a chance.

That's not a problem for Revision instances, but is for raw revisions. Probably not even a problem there (revisions are generally effectively ASCII in most SCMs), but it's more of a problem when forcing the encoding for filenames (which are completely at the whim of the developers committing code).

Making a change to specifically check for and cast the Revision before storing it instead, which should be a bit safer.

Reply all
Reply to author
Forward
0 new messages