Skip to content

Some String Has Truncated Before Model Save In Releases Version==3.2.3 #1543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
langdangren opened this issue Dec 14, 2021 · 8 comments · Fixed by #1547
Closed

Some String Has Truncated Before Model Save In Releases Version==3.2.3 #1543

langdangren opened this issue Dec 14, 2021 · 8 comments · Fixed by #1547

Comments

@langdangren
Copy link

I found some string was truncated in this function

def strip_GeomFromEWKB(param):
    if isinstance(param, str):
        return param.lstrip("ST_GeomFromEWKB('\\x").rstrip("'::bytea)")
    return param

Related Pull Request

Minimally Reproducible Test Case

In [8]: "botpy".lstrip("ST_GeomFromEWKB('\\x").rstrip("'::bytea)")
Out[8]: 'botp'

In [9]: "Botpy".lstrip("ST_GeomFromEWKB('\\x").rstrip("'::bytea)")
Out[9]: 'tp'
@langdangren langdangren changed the title Some String Has Truncated Before Model Save In Releases==3.2.3 Some String Has Truncated Before Model Save In Releases Version==3.2.3 Dec 14, 2021
@tim-schilling
Copy link
Member

@tryReboot do you have an example of a query that is impacted by this?

@langdangren
Copy link
Author

@tim-schilling For example , you want to save a string into some django model, if this string include "y" or "e", this string was truncated then save into database. I don't know if anyone else has same problem.

@tim-schilling
Copy link
Member

It sounds like the issue is when the parameter is not for a GIS query and it's stripping these characters still.

@bellini666
Copy link
Contributor

Hey guys,

Wow, was getting crazy about this... Have some constraints that check that some CharField with choices has their value in exactly one of the choices, but the tests where passing fine, creating/updating the model in a shell was working fine, but when creating it in a request (django-admin/any other view) gave me the error. Examining the sql I notice that some characters were missing, for example "ecommerce" was appearing on sql as "commerc", "flat" as "fl", and even some names like "John Doe" as "John Do".

I don't use GIS in that specific project so it is not related to only GIS. I think it is important to give this a _very high priority on releasing a bugfix since someone could have the debug-toolbar enabled in a production code (even if in the middleware only), and he would for sure have issues with his data.

The problem caught my attention because, in the example above, "ecommerce" is an enum for me and I have a database constraint that checks this and don't let me type anything other than the valid values, and "commerc" is not one of those. But the fact that "John Doe" was written as "John Do" could easily pass silently. And like I said, unit tests are unlikely to ge this unless they are really testing the request itself (for me they didn't, specially because even when testing the requests, I only enable the toolbar's middleware for code running on manage.py itself)

@bellini666
Copy link
Contributor

Also, I saw that last release was released as 3.2.3. Why not release it as 3.3?

By only increasing the micro it looks like it is a release that only has bugfixes. By increasing at least the minor it makes it easy for people to know that "well, let me take a look at the changelog to check if there's something that I need to be aware of".

tim-schilling added a commit that referenced this issue Dec 15, 2021
The stripping logic is stripping general sql parameters and the test
written for it does not run. I'm not confident that the test actually
hits the code we expect it to run either.

Closes #1543
Reopens #423
@tim-schilling
Copy link
Member

Also, I saw that last release was released as 3.2.3. Why not release it as 3.3?

As far as I knew, there were no issues with the project and it was all bug fixes.

tim-schilling added a commit that referenced this issue Dec 15, 2021
The stripping logic is stripping general sql parameters and the test
written for it does not run. I'm not confident that the test actually
hits the code we expect it to run either.

Closes #1543
Reopens #423
@tim-schilling
Copy link
Member

@bellini666 thank you for escalating the issue. I likely would have waited until tomorrow to release 3.2.4 otherwise.

@bellini666
Copy link
Contributor

Hey @tim-schilling , np! Ty for acting that fast! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants