encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
28.21k stars 6.81k forks source link

3.15.2 includes two undocumented breaking changes #9450

Open cliebBS opened 3 months ago

cliebBS commented 3 months ago

In the 3.15.2 release, two very large breaking changes were included:

These were both breaking changes released as part of a bugfix release, which included a fix for a security issue, and which are not even documented in the release notes. Examining your past releases shows that you normally only do breaking changes like these ones in major and minor versions. At a minimum, the changelog should be updated to clearly call out that all of these releases are no longer supported.

lampwins commented 3 months ago

Not to get too philosophical, but this sort of thing is a major reason why people complain so much about the python packaging ecosystem. I know DRF does not state they use semantic versioning, but the documented versioning strategy does state for these "minor releases":

You should be able to upgrade between minor point releases without any other code changes.

If it is not painfully obvious, the modern python package tooling landscape does rely heavily on adhering to something that resembles the meaning of semantic versioning, when using a major.minor.patch like scheme. It's even more important for such an ubiquidous project within the django ecosystem.

I'm not trying to be mean, but I really hope that a project like DRF can lead by example.

onegreyonewhite commented 3 months ago

Not to get too philosophical, but this sort of thing is a major reason why people complain so much about the python packaging ecosystem. I know DRF does not state they use semantic versioning, but the documented versioning strategy does state for these "minor releases":

You should be able to upgrade between minor point releases without any other code changes.

If it is not painfully obvious, the modern python package tooling landscape does rely heavily on adhering to something that resembles the meaning of semantic versioning, when using a major.minor.patch like scheme. It's even more important for such an ubiquidous project within the django ecosystem.

I'm not trying to be mean, but I really hope that a project like DRF can lead by example.

Damn, they just released 3.15.0 (it should be beta) because everyone was complaining and eagerly waiting for this release. The issue is that some pull requests should have been merged earlier since they promised to include them in this release. In fact, 3.15 was already supposed to drop support for older Python and Django versions.

As far as I remember, during the discussion about ending support for Python 3.6, @tomchristie suggested a great idea: to stick to the supported Django releases and their corresponding Python versions at the time of the release.

Everyone who follows the project and was preparing for the release has already updated their projects to the current versions. But I agree that there's nothing wrong with bumping the minor version every time a version of Python or Django is dropped. Especially since the project has moved into the stabilization phase and will be focusing more on maintenance rather than introducing new features.

cliebBS commented 3 months ago

At a bare minimum, this should have been mentioned in the release notes for the release. My primary issue is that this is a stealth change to anyone who doesn't follow the internals of the development process. Plenty of projects (annoyingly) don't follow semantic versioning.

It sounds reasonable to drop support in the next minor version after Django drops support. This feels very much like how the Scientific Python community has chosen to handle Python support, where numpy drives the version of Python that everyone else supports, and they have a clear policy on which Python versions they will support. This makes things easy for us consumers of the library as we can know what to expect just by following the consistent release schedules of a couple of tentpole projects.

On Wed, Jun 26, 2024, 3:25 PM Sergei Kliuikov @.***> wrote:

Not to get too philosophical, but this sort of thing is a major reason why people complain so much about the python packaging ecosystem. I know DRF does not state they use semantic versioning, but the documented versioning strategy https://www.django-rest-framework.org/community/release-notes/#versioning does state for these "minor releases":

You should be able to upgrade between minor point releases without any other code changes.

If it is not painfully obvious, the modern python package tooling landscape does rely heavily on adhering to something that resembles the meaning of semantic versioning, when using a major.minor.patch like scheme. It's even more important for such an ubiquidous project within the django ecosystem.

I'm not trying to be mean, but I really hope that a project like DRF can lead by example.

Damn, they just released 3.15.0 (it should be beta) because everyone was complaining and eagerly waiting for this release. The issue is that some pull requests should have been merged earlier since they promised to include them in this release. In fact, 3.15 was already supposed to drop support for older Python and Django versions.

As far as I remember, during the discussion about ending support for Python 3.6, @tomchristie https://github.com/tomchristie suggested a great idea: to stick to the supported Django releases and their corresponding Python versions at the time of the release.

Everyone who follows the project and was preparing for the release has already updated their projects to the current versions. But I agree that there's nothing wrong with bumping the minor version every time a version of Python or Django is dropped. Especially since the project has moved into the stabilization phase and will be focusing more on maintenance rather than introducing new features.

— Reply to this email directly, view it on GitHub https://github.com/encode/django-rest-framework/issues/9450#issuecomment-2192472487, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFFO7AT3ATINWAL37NV5DDLZJMISJAVCNFSM6AAAAABJ54PA5CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJSGQ3TENBYG4 . You are receiving this because you authored the thread.Message ID: @.***>

lampwins commented 3 months ago

Everyone who follows the project and was preparing for the release has already updated their projects to the current versions. But I agree that there's nothing wrong with bumping the minor version every time a version of Python or Django is dropped. Especially since the project has moved into the stabilization phase and will be focusing more on maintenance rather than introducing new features.

Exactly. It's all well and good to put such policies and procedures in place, but the adherence to versioning norms should still take place. So folks that are not closely watching the communications around a project can still safely rely on the versioning scheme and the broader python ecosystem conventions to not blow things up. Thats all. Much love for DRF still.

tomchristie commented 3 months ago

Ah okay, this will have been in error as a result of time pressure dealing with security related fix "Fix potential XSS vulnerability in browsable API." #9435

We haven't actually made any functional changes that prevent Python 3.6 / Django 3.0. See https://github.com/encode/django-rest-framework/commit/1f2daaf53cb1e62080be99ea15986f607a193817 Although the setup.py does exclude them.

I think what we should probably do here to minimise disruption is...

TomNewChao commented 2 months ago

Ah okay, this will have been in error as a result of time pressure dealing with security related fix "Fix potential XSS vulnerability in browsable API." #9435

We haven't actually made any functional changes that prevent Python 3.6 / Django 3.0. See 1f2daaf Although the setup.py does exclude them.

I think what we should probably do here to minimise disruption is...

  • Rollback the setup.py changes with a 3.15.3 release.
  • The security fix ought to be highlighted with Security: ... in the release notes, as with two other occurrences there.

Yeah, You are right. To resolve CVE-2024-21520, I upgraded django-rest-framework to 3.15.2, but django-rest-framework requires django to be installed later than 3.2.25, which forced me to upgrade django. This is really fucked up.

browniebroke commented 2 months ago

Django 3.2 is no longer maintained and has probably a few CVEs too. Perhaps upgrade that first?

TomNewChao commented 2 months ago

Django 3.2 is no longer maintained and has probably a few CVEs too. Perhaps upgrade that first?

Currently, Django 3.2.25 has not scanned for CVE vulnerabilities. This version was released 4 months ago. For details, see: https://github.com/django/django/commits/3.2.25/. If CVE vulnerabilities are found in the future, we will consider upgrading.

browniebroke commented 2 months ago

Might want to look whether 3.2 has the vulnerabilities from https://www.djangoproject.com/weblog/2024/jul/09/security-releases/

I checked the code from the 3.2 branch and at least one of them is present. So... Time to upgrade, I guess?

omasback commented 2 months ago
  • Rollback the setup.py changes with a 3.15.3 release.

Is this going to happen? Not trying to rush you – just looking for guidance.

TomNewChao commented 2 months ago

Might want to look whether 3.2 has the vulnerabilities from https://www.djangoproject.com/weblog/2024/jul/09/security-releases/

I checked the code from the 3.2 branch and at least one of them is present. So... Time to upgrade, I guess?

glad to receive your reply, but I used trivy tool to scan Django 3.2.25 version, but did not find any vulnerability hints.

cliebBS commented 2 months ago

glad to receive your reply, but I used trivy tool to scan Django 3.2.25 version, but did not find any vulnerability hints.

This is most likely because the CVEs that were issued for these vulns were only issued for the currently supported versions of Django. I went through each of the MRs for each of the CVEs (they are linked at the bottom of the article) and verified that all of them exist in 3.2.25, so the CVEs are just as valid for 3.2.25 as they are for the unpatched versions of 4.2 and 5.0.

TomNewChao commented 2 months ago

glad to receive your reply, but I used trivy tool to scan Django 3.2.25 version, but did not find any vulnerability hints.

This is most likely because the CVEs that were issued for these vulns were only issued for the currently supported versions of Django. I went through each of the MRs for each of the CVEs (they are linked at the bottom of the article) and verified that all of them exist in 3.2.25, so the CVEs are just as valid for 3.2.25 as they are for the unpatched versions of 4.2 and 5.0.

ok, got it, Let's plan to upgrade Django.

ryanisnan commented 1 month ago

Thanks for the response to this ticket. I want to add an additional request here that DRF maintains compliance with its own guidance as mentioned here. Needing to scrutinize minor version bumps, on top of painful medium version bumps is not ideal.

As always, take this feedback with a grain of salt. We are very thankful for the efforts of this community.

tomchristie commented 1 month ago

@encode/django-rest-framework Okay, suppose we roll a 3.15.3 release without the setup.py changes. And then do the version upgrades in a 3.16.0 release. In line with previous releases.

Is there anything else that ought to make it in to 3.15.3 to minimised any other 3.14.x upgrade disruptions?

(Apologies all for the churn this time around, it's not been ideal but we do at least have a handle on getting things settled down now.)

peterthomassen commented 3 weeks ago

Is there anything else that ought to make it in to 3.15.3 to minimised any other 3.14.x upgrade disruptions?

I think the following regressions still need to be addressed: #9358, #9378, #9410.