Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,6 @@ jobs:
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v6
- name: Install Selenium
run: |
curl -LsSfO https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb
sudo dpkg -i google-chrome-stable_current_amd64.deb || sudo apt-get -f install -y
if: matrix.os == 'ubuntu-latest'
- uses: astral-sh/setup-uv@v7
- run: uv run pytest -m selenium
- uses: codecov/codecov-action@v5
Expand Down
10 changes: 9 additions & 1 deletion s3file/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.templatetags.static import static
from django.utils.functional import cached_property
from django.utils.html import format_html, html_safe
from django.utils.safestring import mark_safe
from storages.utils import safe_join

from s3file.middleware import S3FileMiddleware
Expand Down Expand Up @@ -75,7 +76,6 @@ def client(self):

def build_attrs(self, *args, **kwargs):
attrs = super().build_attrs(*args, **kwargs)
attrs["is"] = "s3-file"

accept = attrs.get("accept")
response = self.client.generate_presigned_post(
Expand All @@ -97,6 +97,14 @@ def build_attrs(self, *args, **kwargs):

return defaults

def render(self, name, value, attrs=None, renderer=None):
"""Render the widget as a custom element for Safari compatibility."""
return mark_safe( # noqa: S308
str(super().render(name, value, attrs=attrs, renderer=renderer)).replace(
f'<input type="{self.input_type}"', "<s3-file"
)
)
Comment on lines +100 to +106
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using mark_safe with string replacement on potentially user-controlled HTML is risky. The parent's render() method produces HTML that could contain user-provided attribute values (like file names or custom attributes). The simple .replace() operation doesn't validate or sanitize the replacement, and if the parent render output contains unexpected patterns (e.g., multiple <input type="file" strings, or injected content that looks like the pattern), the replacement could produce malformed or exploitable HTML. While the immediate risk is low since Django's form rendering should be safe, consider using proper HTML parsing with a library like html.parser or BeautifulSoup for more robust transformation.

Copilot uses AI. Check for mistakes.
Comment on lines +100 to +106
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The render method uses a simple string replacement that only changes the opening tag from <input type="file" to <s3-file, but doesn't properly handle the closing tag. HTML file input elements are self-closing (<input ... />), but the custom element <s3-file> needs a proper closing tag (</s3-file>). This replacement will produce malformed HTML like <s3-file ... /> instead of <s3-file ...></s3-file>. Consider using proper HTML parsing and reconstruction, or at minimum, also replace the closing portion of the tag (e.g., /> with ></s3-file>).

Copilot uses AI. Check for mistakes.

def get_conditions(self, accept):
conditions = [
{"bucket": self.bucket_name},
Expand Down
223 changes: 206 additions & 17 deletions s3file/static/s3file/js/s3file.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Parse XML response from AWS S3 and return the file key.
*
* @param {string} responseText - XML response form AWS S3.
* @param {string} responseText - XML response from AWS S3.
* @return {string} - Key from response.
*/
export function getKeyFromResponse(responseText) {
Expand All @@ -11,33 +11,201 @@ export function getKeyFromResponse(responseText) {

/**
* Custom element to upload files to AWS S3.
* Safari-compatible autonomous custom element that acts as a file input.
*
* @extends HTMLInputElement
* @extends HTMLElement
*/
export class S3FileInput extends globalThis.HTMLInputElement {
export class S3FileInput extends globalThis.HTMLElement {
static passThroughAttributes = ["accept", "required", "multiple", "class", "style"]
constructor() {
super()
this.type = "file"
this.keys = []
this.upload = null
this._files = []
this._validationMessage = ""
this._internals = null

// Try to attach ElementInternals for form participation
try {
this._internals = this.attachInternals?.()
} catch (e) {
// ElementInternals not supported
}
}

connectedCallback() {
this.form.addEventListener("formdata", this.fromDataHandler.bind(this))
this.form.addEventListener("submit", this.submitHandler.bind(this), { once: true })
this.form.addEventListener("upload", this.uploadHandler.bind(this))
this.addEventListener("change", this.changeHandler.bind(this))
// Create a hidden file input for the file picker functionality
this._fileInput = document.createElement("input")
this._fileInput.type = "file"

// Sync attributes to hidden input
this._syncAttributesToHiddenInput()

// Listen for file selection on hidden input
this._fileInput.addEventListener("change", () => {
this._files = this._fileInput.files
this.dispatchEvent(new Event("change", { bubbles: true }))
this.changeHandler()
})

// Append elements
this.appendChild(this._fileInput)

// Setup form event listeners
this.form?.addEventListener("formdata", this.fromDataHandler.bind(this))
this.form?.addEventListener("submit", this.submitHandler.bind(this), {
once: true,
})
this.form?.addEventListener("upload", this.uploadHandler.bind(this))
}

/**
* Sync attributes from custom element to hidden input.
*/
_syncAttributesToHiddenInput() {
if (!this._fileInput) return

S3FileInput.passThroughAttributes.forEach((attr) => {
if (this.hasAttribute(attr)) {
this._fileInput.setAttribute(attr, this.getAttribute(attr))
} else {
this._fileInput.removeAttribute(attr)
}
})

this._fileInput.disabled = this.hasAttribute("disabled")
}

/**
* Implement HTMLInputElement-like properties.
*/
get files() {
return this._files
}
Comment on lines +82 to +84
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The files getter returns this._files which is initialized as an empty array [] in the constructor, but native file input elements return a FileList object, not an array. While both are iterable, they have different APIs - FileList has an item() method and behaves differently in some contexts. This type mismatch could cause issues with code that expects a true FileList object. Consider initializing this._files to an empty FileList-like object or document this limitation.

Copilot uses AI. Check for mistakes.

get type() {
return "file"
}

get name() {
return this.getAttribute("name") || ""
}

set name(value) {
this.setAttribute("name", value)
}

get value() {
if (this._files && this._files.length > 0) {
return this._files[0].name
}
return ""
}

set value(val) {
// Setting value on file inputs is restricted for security
if (val === "" || val === null) {
this._files = []
if (this._fileInput) {
this._fileInput.value = ""
}
}
}

get form() {
return this._internals?.form || this.closest("form")
}

get disabled() {
return this.hasAttribute("disabled")
}

set disabled(value) {
if (value) {
this.setAttribute("disabled", "")
} else {
this.removeAttribute("disabled")
}
}

get required() {
return this.hasAttribute("required")
}

set required(value) {
if (value) {
this.setAttribute("required", "")
} else {
this.removeAttribute("required")
}
}

get validity() {
if (this._internals) {
return this._internals.validity
}
// Create a basic ValidityState-like object
const isValid = !this.required || (this._files && this._files.length > 0)
return {
valid: isValid && !this._validationMessage,
valueMissing: this.required && (!this._files || this._files.length === 0),
customError: !!this._validationMessage,
badInput: false,
patternMismatch: false,
rangeOverflow: false,
rangeUnderflow: false,
stepMismatch: false,
tooLong: false,
tooShort: false,
typeMismatch: false,
}
}
Comment on lines +143 to +162
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The custom ValidityState object in the validity getter doesn't properly reflect the state when using ElementInternals. When this._internals exists, it returns this._internals.validity, but the _validationMessage property updates are not reflected in that ValidityState object. The setCustomValidity method updates both _validationMessage and _internals.setValidity, but the fallback validity getter only checks _validationMessage. This creates an inconsistency where the validity state might not match the validation message when ElementInternals is available. Consider always checking both sources or ensuring they stay in sync.

Copilot uses AI. Check for mistakes.

get validationMessage() {
return this._validationMessage
}

setCustomValidity(message) {
this._validationMessage = message || ""
if (this._internals && typeof this._internals.setValidity === "function") {
if (message) {
this._internals.setValidity({ customError: true }, message)
} else {
this._internals.setValidity({})
}
}
}

reportValidity() {
const validity = this.validity
if (validity && !validity.valid) {
this.dispatchEvent(new Event("invalid", { bubbles: false, cancelable: true }))
return false
}
return true
}

checkValidity() {
return this.validity.valid
}

click() {
if (this._fileInput) {
this._fileInput.click()
}
}

changeHandler() {
this.keys = []
this.upload = null
try {
this.form.removeEventListener("submit", this.submitHandler.bind(this))
this.form?.removeEventListener("submit", this.submitHandler.bind(this))
} catch (error) {
console.debug(error)
}
this.form.addEventListener("submit", this.submitHandler.bind(this), { once: true })
this.form?.addEventListener("submit", this.submitHandler.bind(this), {
once: true,
})
}

/**
Expand All @@ -48,15 +216,15 @@ export class S3FileInput extends globalThis.HTMLInputElement {
*/
async submitHandler(event) {
event.preventDefault()
this.form.dispatchEvent(new window.CustomEvent("upload"))
await Promise.all(this.form.pendingRquests)
this.form.requestSubmit(event.submitter)
this.form?.dispatchEvent(new window.CustomEvent("upload"))
await Promise.all(this.form?.pendingRquests)
this.form?.requestSubmit(event.submitter)
}

uploadHandler() {
if (this.files.length && !this.upload) {
this.upload = this.uploadFiles()
this.form.pendingRquests = this.form.pendingRquests || []
this.form.pendingRquests = this.form?.pendingRquests || []
this.form.pendingRquests.push(this.upload)
}
}
Expand Down Expand Up @@ -99,7 +267,10 @@ export class S3FileInput extends globalThis.HTMLInputElement {
s3Form.append("file", file)
console.debug("uploading", this.dataset.url, file)
try {
const response = await fetch(this.dataset.url, { method: "POST", body: s3Form })
const response = await fetch(this.dataset.url, {
method: "POST",
body: s3Form,
})
if (response.status === 201) {
this.keys.push(getKeyFromResponse(await response.text()))
} else {
Expand All @@ -108,11 +279,29 @@ export class S3FileInput extends globalThis.HTMLInputElement {
}
} catch (error) {
console.error(error)
this.setCustomValidity(error)
this.setCustomValidity(String(error))
this.reportValidity()
}
}
}

/**
* Called when observed attributes change.
*/
static get observedAttributes() {
return this.passThroughAttributes.concat(["name", "id"])
}

attributeChangedCallback(name, oldValue, newValue) {
this._syncAttributesToHiddenInput()
}

/**
* Declare this element as a form-associated custom element.
*/
static get formAssociated() {
return true
}
}

globalThis.customElements.define("s3-file", S3FileInput, { extends: "input" })
globalThis.customElements.define("s3-file", S3FileInput)
5 changes: 2 additions & 3 deletions tests/__tests__/s3file.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ describe("getKeyFromResponse", () => {
describe("S3FileInput", () => {
test("constructor", () => {
const input = new s3file.S3FileInput()
assert.strictEqual(input.type, "file")
assert.deepStrictEqual(input.keys, [])
assert.strictEqual(input.upload, null)
})
Expand All @@ -35,11 +34,11 @@ describe("S3FileInput", () => {
const form = document.createElement("form")
document.body.appendChild(form)
const input = new s3file.S3FileInput()
input.addEventListener = mock.fn(input.addEventListener)
form.addEventListener = mock.fn(form.addEventListener)
form.appendChild(input)
assert(form.addEventListener.mock.calls.length === 3)
assert(input.addEventListener.mock.calls.length === 1)
assert(input._fileInput !== null)
assert(input._fileInput.type === "file")
})

test("changeHandler", () => {
Expand Down
Loading
Loading