Skip to content

Commit

Permalink
More strict handling of requirement specifiers in requirements.txt. (#…
Browse files Browse the repository at this point in the history
…5118)

* More strict handling of requirement specifiers in requirements.txt.

After some discussion, the runtime team has decided that we should
prevent users from specifying version identifiers altogether within
requirements.txt, rather than silently dropping them. This would
prevent users from being deluded into thinking that they could
specify versions of Python packages in Python workers.

* Rethrow UserError in catch handler

* small fixes
  • Loading branch information
garrettgu10 authored Mar 4, 2024
1 parent 8935526 commit 30694a3
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/swift-peaches-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": minor
---

fix: Including version identifiers in Python requirements.txt will now throw an error
44 changes: 29 additions & 15 deletions packages/wrangler/src/deployment-bundle/find-additional-modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ async function* getFiles(
}
}

/**
* Checks if a given string is a valid Python package identifier.
* See https://packaging.python.org/en/latest/specifications/name-normalization/
* @param name The package name to validate
*/
function isValidPythonPackageName(name: string): boolean {
const regex = /^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$/i;
return regex.test(name);
}

/**
* Search the filesystem under the `moduleRoot` of the `entry` for potential additional modules
* that match the given `rules`.
Expand All @@ -55,30 +65,34 @@ export async function findAdditionalModules(
getBundleType(entry.format, entry.file) === "python";

if (isPythonEntrypoint) {
let pythonRequirements = "";
try {
const pythonRequirements = await readFile(
pythonRequirements = await readFile(
path.resolve(entry.directory, "requirements.txt"),
"utf-8"
);

// This is incredibly naive. However, it supports common syntax for requirements.txt
for (const requirement of pythonRequirements.split("\n")) {
const packageName = requirement.match(/^[^\d\W]\w*/);
if (typeof packageName?.[0] === "string") {
modules.push({
type: "python-requirement",
name: packageName?.[0],
content: "",
filePath: undefined,
});
}
}
// We don't care if a requirements.txt isn't found
} catch (e) {
// We don't care if a requirements.txt isn't found
logger.debug(
"Python entrypoint detected, but no requirements.txt file found."
);
}

for (const requirement of pythonRequirements.split("\n")) {
if (requirement === "") continue;
if (!isValidPythonPackageName(requirement)) {
throw new UserError(
`Invalid Python package name "${requirement}" found in requirements.txt. Note that requirements.txt should contain package names only, not version specifiers.`
);
}

modules.push({
type: "python-requirement",
name: requirement,
content: "",
filePath: undefined,
});
}
}
if (modules.length > 0) {
logger.info(`Attaching additional modules:`);
Expand Down

0 comments on commit 30694a3

Please sign in to comment.