search

heroku / heroku-buildpack-python

Heroku's buildpack for Python applications.
https://www.heroku.com/python
MIT License
976 stars 3 forks source link

Issues with the vendored sqlite/libsqlite #427

Closed edmorley closed 6 years ago

edmorley commented 7 years ago

Some context:

There are a few problems with this:

As such, I propose switching from vendoring sqlite to using the OS packages. I'm guessing the cedar-14 images will want to be left as-is, and so this only be done for Heroku-16.

This would require: 1) Adding libsqlite3-dev to the heroku:16-build image. 2) Adding sqlite3 (the command line tool) to the heroku:16 image, for parity with the vendored archive. (Whilst it's not needed to build Python, I'm presuming there are people relying on it.) 3) Making the sqlite build script return early for heroku-16 (the same as for libmemcached), since bob-builder doesn't support per-platform dependencies.

@kennethreitz, thoughts? I'm happy to open the necessary stack-images PR if you think this makes sense.

kennethreitz commented 7 years ago

/cc @dzuelke. Thoughts on adding libsqlite3-dev to the heroku:16-build image?

kennethreitz commented 7 years ago

I don't think this will change any time soon, but I'm also interested in hearing David's thoughts on this.

dzuelke commented 7 years ago

The reason it's not there is historical: to prevent Rails apps from building with a "local DB" sqlite gem. I think we should revisit this in the interest of consistency. /cc @hone @schneems

schneems commented 7 years ago

Having sqlite on the platform would make detecting an extremely common "wrong" thing that Rails devs do much harder (maybe impossible).

edmorley commented 7 years ago

@schneems, I presume you're referring to this? https://github.com/heroku/heroku-buildpack-ruby/blob/ef2a0a9f14ebb9ec6e9e902db5c8058af2ffc164/lib/language_pack/ruby.rb#L633-L639

Could this case be detected by grepping the gem install log for 'sqlite', or else running gem list and grepping that?

Also, if "prevent people from accidentally using sqlite on an ephemeral filesystem" is a goal, then perhaps the better direction is to stop supporting it on Python too? (Given Django defaults to using sqlite as well.) If sqlite isn't available during Python compile, the compile still succeeds but just means that at runtime an import sqlite3 will fail.

schneems commented 7 years ago

@edmorley we can already parse the Gemfile.lock and we know exactly which gems are installed. The issue is that a minority of users have installed sqlite3 via a buildpack and actually use it for some valid reason, so if we detect the gem and fail we will be preventing valid users from doing a valid thing on Heroku.

kennethreitz commented 7 years ago

sqlite is part of the standard library, and is expected to be importable for certian libraries — for example, the every-important iPython. Therefore, Python supports it.

edmorley commented 7 years ago

Another option here would be to use an approach similar to heroku-buildpack-apt and create temporary APT cache/state directories, allowing apt-get update and then apt-get download as non-root, to download the .deb package for libsqlite3-dev as part of the Python build script. This would then be extracted and referenced during the build, and deleted after.

If this sounds acceptable, I could try putting together an example for the new Python 2.7.14 release (doing this as part of a new version release is probably best; it means people have to opt into it).

kennethreitz commented 6 years ago

@edmorley closing this until you make progress on it :)

edmorley commented 6 years ago

I may not have time to double back to this for a while unfortunately. However it's worth emphasising:

The vendored sqlite is extremely old (v3.7.9, released on 2011-11-01). Compare this to v3.11.0 on Xenial and v3.8.2 on Trusty.

Along with: https://www.cvedetails.com/vendor/9237/Sqlite.html