Skip to content

Fix SQLite#812

Closed
KevinBrolly wants to merge 2 commits intoheroku:masterfrom
KevinBrolly:fix-sqlite
Closed

Fix SQLite#812
KevinBrolly wants to merge 2 commits intoheroku:masterfrom
KevinBrolly:fix-sqlite

Conversation

@KevinBrolly
Copy link
Copy Markdown
Contributor

Context:

Ed Morely first brought up issues with how SQLite3 was being vendored into the buildpack in the following issue - #427

The buildpack was using a very old (v3.7.9, released on 2011-11-01) version of SQLite3 which had some vulnerabilities - https://www.cvedetails.com/vendor/9237/Sqlite.html

The vendored sqlite archive included three components, libsqlite3-0, libsqlite3-dev, and the sqlite3 CLI binary.

Python itself only has a run-time dependency on libsqlite3-0 and a build-time dependency on libsqlite3-dev, the sqlite3 CLI binary is not needed for any python-specific functionality.

@hone implemented a solution (#713) which installs the SQLite headers when building runtimes, and installs the headers and binary as a step in the build process while using libsqlite3-0 that is provided by the stack. This solution works but is quite complicated, and recently cause an issue with cedar-14 builds - c361c2f

We could achieve a similar outcome by installing libsqlite3-dev in the docker container used for runtime builds, making it available as required while python is being built, then we can rely on the stack provided libsqlite3-0 during runtime, removing a step during runtime builds and app builds, as well as a whole lot of code.

The only downside of this approach is the loss of the sqlite CLI binary, but I cannot think why this would be a necessary inclusion, and could be installed if required by individual applications using something like Heroku Buildpack apt - https://github.com/heroku/heroku-buildpack-apt

I made this a draft PR currently as a few more steps would be required to make it work. There is currently no Dockerfile for cedar-14 and it appears runtime builds are currently being done in-dyno, which would make this solution not work for cedar-14, however runtime builds could be moved to Docker for cedar-14 or we could wait until the cedar-14 deprecation next month.

Additionally, the python runtime is currently being cached as part of the build step, so we would need to invalidate the cache to allow users to get the new runtimes once built.

@edmorley
Copy link
Copy Markdown
Member

We could achieve a similar outcome by installing libsqlite3-dev in the docker container used for runtime builds, making it available as required while python is being built

Ah great idea - doesn't require the headers to be in the stack image build variant (avoiding the concerns in #427 (comment)), without having to resort to a fragile DIY apt approach :-)

@CaseyFaist CaseyFaist mentioned this pull request Sep 27, 2019
@CaseyFaist CaseyFaist closed this Mar 18, 2020
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 this pull request may close these issues.

3 participants