Merge pull request 'Use binary screenshot storage and upload end to end' (#492) from timmy/491-binary-screenshot-transport into main
This commit is contained in:
commit
4749d15d85
|
|
@ -26,10 +26,11 @@ admit their text and image bytes to IndexedDB before confirmation, keep only bou
|
|||
localStorage, and use checkpointed upload/comment identities so reconnect retries cannot duplicate
|
||||
either stage. The mobile **New issue**
|
||||
sheet accepts the same image formats and stores the screenshot with its account-bound
|
||||
outbox capture. Durable admission writes the complete screenshot capture to IndexedDB
|
||||
outbox capture. Durable admission writes the complete screenshot capture as a binary Blob to IndexedDB
|
||||
before confirmation; localStorage keeps only bounded attachment metadata, avoiding base64
|
||||
quota pressure and synchronous multi-megabyte writes. Existing queued screenshot payloads
|
||||
migrate to the IndexedDB-backed form on the next durable admission or worker reconciliation.
|
||||
quota pressure and synchronous multi-megabyte writes. Online and background delivery send the
|
||||
original bytes as multipart form data, avoiding the roughly 33% base64 wire expansion. Existing
|
||||
queued base64 screenshot payloads remain readable and are converted only at delivery time.
|
||||
Delivery creates the issue exactly once, then uploads and comments with
|
||||
the image; after a partial failure, retry resumes with the confirmed issue instead of
|
||||
creating a duplicate.
|
||||
|
|
|
|||
|
|
@ -106,7 +106,8 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync,
|
|||
attachment: {
|
||||
filename: String(message.attachment.filename || ''),
|
||||
contentType: String(message.attachment.contentType || ''),
|
||||
data: String(message.attachment.data || ''),
|
||||
...(message.attachment.blob ? { blob: message.attachment.blob } :
|
||||
{ data: String(message.attachment.data || '') }),
|
||||
},
|
||||
} : candidate);
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -77,8 +77,10 @@ function createIssueSyncStore({
|
|||
}
|
||||
continue;
|
||||
}
|
||||
if (current.operationId === replacement.operationId && current.attachment?.data &&
|
||||
replacement.attachment?.stored && !replacement.attachment.data) {
|
||||
if (current.operationId === replacement.operationId &&
|
||||
(current.attachment?.data || current.attachment?.blob) &&
|
||||
replacement.attachment?.stored &&
|
||||
!replacement.attachment.data && !replacement.attachment.blob) {
|
||||
replacement = { ...replacement, attachment: current.attachment };
|
||||
incoming.set(current.id, replacement);
|
||||
}
|
||||
|
|
@ -153,7 +155,8 @@ function createIssueSyncStore({
|
|||
if (current && ((current.status === 'sending' && Number(current.claimUntil) > Number(now())) ||
|
||||
(current.status === 'attention' && item.status === 'attention') ||
|
||||
current.status === 'sent')) return current;
|
||||
const preservedAttachment = current?.attachment?.data && item?.attachment?.stored && !item.attachment.data
|
||||
const preservedAttachment = (current?.attachment?.data || current?.attachment?.blob) &&
|
||||
item?.attachment?.stored && !item.attachment.data && !item.attachment.blob
|
||||
? { attachment: current.attachment } : {};
|
||||
const next = current && current.operationId === item.operationId ? {
|
||||
...item, ...preservedAttachment,
|
||||
|
|
@ -417,6 +420,19 @@ function createBackgroundIssueSync({
|
|||
return String(operationId || '').slice(0, 128 - suffix.length) + suffix;
|
||||
}
|
||||
|
||||
function attachmentMultipart(attachment) {
|
||||
let blob = attachment?.blob;
|
||||
if (!blob && attachment?.data) {
|
||||
const binary = atob(String(attachment.data));
|
||||
const bytes = Uint8Array.from(binary, character => character.charCodeAt(0));
|
||||
blob = new Blob([bytes], { type: String(attachment.contentType || '') });
|
||||
}
|
||||
if (!blob) throw new Error('The saved screenshot is unavailable. Retry before sending.');
|
||||
const form = new FormData();
|
||||
form.append('file', blob, String(attachment.filename || 'screenshot'));
|
||||
return form;
|
||||
}
|
||||
|
||||
async function deliverIssueCapture(item) {
|
||||
const repository = String(item.repository || '').split('/').map(encodeURIComponent).join('/');
|
||||
let deliveredIssue = item.deliveredIssue;
|
||||
|
|
@ -433,14 +449,10 @@ function createBackgroundIssueSync({
|
|||
{
|
||||
method: 'POST',
|
||||
headers: {
|
||||
Accept: 'application/json', 'Content-Type': 'application/json',
|
||||
Accept: 'application/json',
|
||||
'Idempotency-Key': stageOperationId(item.operationId, 'attachment'),
|
||||
},
|
||||
body: JSON.stringify({
|
||||
filename: item.attachment.filename,
|
||||
content_type: item.attachment.contentType,
|
||||
data: item.attachment.data,
|
||||
}),
|
||||
body: attachmentMultipart(item.attachment),
|
||||
},
|
||||
);
|
||||
attachmentMarkdown = String(uploaded?.markdown || '');
|
||||
|
|
@ -476,14 +488,10 @@ function createBackgroundIssueSync({
|
|||
{
|
||||
method: 'POST',
|
||||
headers: {
|
||||
Accept: 'application/json', 'Content-Type': 'application/json',
|
||||
Accept: 'application/json',
|
||||
'Idempotency-Key': stageOperationId(item.operationId, 'attachment'),
|
||||
},
|
||||
body: JSON.stringify({
|
||||
filename: item.attachment.filename,
|
||||
content_type: item.attachment.contentType,
|
||||
data: item.attachment.data,
|
||||
}),
|
||||
body: attachmentMultipart(item.attachment),
|
||||
},
|
||||
);
|
||||
attachmentMarkdown = String(uploaded?.markdown || '');
|
||||
|
|
|
|||
|
|
@ -296,14 +296,9 @@
|
|||
method: 'POST',
|
||||
headers: {
|
||||
Accept: 'application/json',
|
||||
'Content-Type': 'application/json',
|
||||
'Idempotency-Key': payload.operation_id,
|
||||
},
|
||||
body: JSON.stringify({
|
||||
filename: payload.filename,
|
||||
content_type: payload.content_type,
|
||||
data: payload.data,
|
||||
}),
|
||||
body: issueAttachment.multipart(payload),
|
||||
},
|
||||
);
|
||||
},
|
||||
|
|
|
|||
|
|
@ -8,8 +8,20 @@
|
|||
const MAX_BYTES = 2 * 1024 * 1024;
|
||||
const IMAGE_TYPES = new Set(['image/png', 'image/jpeg', 'image/webp']);
|
||||
|
||||
function multipart(attachment) {
|
||||
let blob = attachment?.blob;
|
||||
if (!blob && attachment?.data) {
|
||||
const binary = atob(String(attachment.data));
|
||||
const bytes = Uint8Array.from(binary, character => character.charCodeAt(0));
|
||||
blob = new Blob([bytes], { type: String(attachment.contentType || '') });
|
||||
}
|
||||
if (!blob) throw new Error('The saved screenshot is unavailable. Retry before sending.');
|
||||
const form = new FormData();
|
||||
form.append('file', blob, String(attachment.filename || 'screenshot'));
|
||||
return form;
|
||||
}
|
||||
|
||||
function create(options) {
|
||||
const readDataUrl = options.readDataUrl;
|
||||
const upload = options.upload;
|
||||
const createOperationId = options.createOperationId || (() => {
|
||||
if (typeof globalThis !== 'undefined' && globalThis.crypto?.randomUUID) {
|
||||
|
|
@ -46,15 +58,16 @@
|
|||
function restore(value) {
|
||||
const contentType = String(value?.contentType || '');
|
||||
const filename = String(value?.filename || '');
|
||||
const blob = value?.blob;
|
||||
const data = String(value?.data || '');
|
||||
if (!data) {
|
||||
if (!blob && !data) {
|
||||
clear();
|
||||
throw new Error('The saved screenshot is unavailable. Retry before editing this issue.');
|
||||
}
|
||||
const padding = (data.match(/=*$/) || [''])[0].length;
|
||||
const size = Math.max(1, Math.floor(data.length * 3 / 4) - padding);
|
||||
select({ name: filename, type: contentType, size });
|
||||
serialized = { filename, contentType, data };
|
||||
const size = blob ? Number(blob.size) : Math.max(1, Math.floor(data.length * 3 / 4) - padding);
|
||||
select({ name: filename, type: contentType, size, ...(blob ? { blob } : {}) });
|
||||
serialized = blob ? { filename, contentType, blob } : { filename, contentType, data };
|
||||
return state();
|
||||
}
|
||||
|
||||
|
|
@ -69,14 +82,10 @@
|
|||
async function serialize() {
|
||||
if (!selected) return null;
|
||||
if (!serialized) {
|
||||
const dataUrl = await readDataUrl(selected);
|
||||
const marker = ';base64,';
|
||||
const markerAt = String(dataUrl).indexOf(marker);
|
||||
if (markerAt < 0) throw new Error('The screenshot could not be read. Choose it again.');
|
||||
serialized = {
|
||||
filename: selected.name,
|
||||
contentType: selected.type,
|
||||
data: String(dataUrl).slice(markerAt + marker.length),
|
||||
blob: selected.blob || selected,
|
||||
};
|
||||
}
|
||||
return { ...serialized };
|
||||
|
|
@ -92,7 +101,7 @@
|
|||
number: item.number,
|
||||
filename: attachment.filename,
|
||||
content_type: attachment.contentType,
|
||||
data: attachment.data,
|
||||
...(attachment.blob ? { blob: attachment.blob } : { data: attachment.data }),
|
||||
operation_id: operationId,
|
||||
});
|
||||
if (!confirmed || typeof confirmed.markdown !== 'string' || !confirmed.markdown) {
|
||||
|
|
@ -144,7 +153,8 @@
|
|||
function restorePreview(value) {
|
||||
clearPreview();
|
||||
const restored = controller.restore(value);
|
||||
previewUrl = 'data:' + value.contentType + ';base64,' + value.data;
|
||||
previewUrl = value.blob ? options.createObjectURL(value.blob) :
|
||||
'data:' + value.contentType + ';base64,' + value.data;
|
||||
options.image.src = previewUrl;
|
||||
options.meta.textContent = restored.name + ' · ' + Math.ceil(restored.size / 1024) + ' KB';
|
||||
options.preview.hidden = false;
|
||||
|
|
@ -155,5 +165,5 @@
|
|||
return Object.assign(controller, { clear: clearPreview, restore: restorePreview });
|
||||
}
|
||||
|
||||
return { create, mount, MAX_BYTES };
|
||||
return { create, mount, multipart, MAX_BYTES };
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,8 +8,10 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge
|
|||
function captureAttachment(value) {
|
||||
const contentType = String(value?.contentType || '');
|
||||
const filename = String(value?.filename || '').slice(0, 255);
|
||||
const blob = value?.blob;
|
||||
const data = String(value?.data || '');
|
||||
if (!filename || !['image/png', 'image/jpeg', 'image/webp'].includes(contentType)) return undefined;
|
||||
if (blob) return { filename, contentType, blob };
|
||||
if (!data && value?.stored === true) return { filename, contentType, stored: true };
|
||||
if (!data) return undefined;
|
||||
return { filename, contentType, data };
|
||||
|
|
@ -61,7 +63,7 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge
|
|||
}
|
||||
|
||||
function localIndexItem(item) {
|
||||
if (!item?.attachment?.data) return item;
|
||||
if (!item?.attachment?.data && !item?.attachment?.blob) return item;
|
||||
return {
|
||||
...item,
|
||||
attachment: {
|
||||
|
|
@ -103,13 +105,13 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge
|
|||
async function hydrateForEdit(id) {
|
||||
const item = read().find(candidate => candidate.id === id);
|
||||
if (!item) return null;
|
||||
if (!item.attachment?.stored || item.attachment.data) return { ...item };
|
||||
if (!item.attachment?.stored || item.attachment.data || item.attachment.blob) return { ...item };
|
||||
if (!backgroundSync?.get) {
|
||||
throw new Error('The saved screenshot is unavailable. Retry before editing this issue.');
|
||||
}
|
||||
const durable = await backgroundSync.get(id);
|
||||
const attachment = captureAttachment(durable?.attachment);
|
||||
if (!attachment?.data || durable?.operationId !== item.operationId) {
|
||||
if ((!attachment?.data && !attachment?.blob) || durable?.operationId !== item.operationId) {
|
||||
throw new Error('The saved screenshot is unavailable. Retry before editing this issue.');
|
||||
}
|
||||
return { ...item, attachment };
|
||||
|
|
@ -196,6 +198,19 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge
|
|||
return String(operationId || '').slice(0, 128 - suffix.length) + suffix;
|
||||
}
|
||||
|
||||
function attachmentMultipart(attachment) {
|
||||
let blob = attachment?.blob;
|
||||
if (!blob && attachment?.data) {
|
||||
const binary = atob(String(attachment.data));
|
||||
const bytes = Uint8Array.from(binary, character => character.charCodeAt(0));
|
||||
blob = new Blob([bytes], { type: String(attachment.contentType || '') });
|
||||
}
|
||||
if (!blob) throw new Error('The saved screenshot is unavailable. Retry before sending.');
|
||||
const form = new FormData();
|
||||
form.append('file', blob, String(attachment.filename || 'screenshot'));
|
||||
return form;
|
||||
}
|
||||
|
||||
async function sendDirect(item, repository) {
|
||||
let issue = item.deliveredIssue;
|
||||
if (!issue) {
|
||||
|
|
@ -221,14 +236,10 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge
|
|||
{
|
||||
method: 'POST',
|
||||
headers: {
|
||||
Accept: 'application/json', 'Content-Type': 'application/json',
|
||||
Accept: 'application/json',
|
||||
'Idempotency-Key': stageOperationId(item.operationId, 'attachment'),
|
||||
},
|
||||
body: JSON.stringify({
|
||||
filename: item.attachment.filename,
|
||||
content_type: item.attachment.contentType,
|
||||
data: item.attachment.data,
|
||||
}),
|
||||
body: attachmentMultipart(item.attachment),
|
||||
},
|
||||
);
|
||||
markdown = String(uploaded?.markdown || '');
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
fastapi==0.133.1
|
||||
httpx==0.28.1
|
||||
pydantic==2.13.4
|
||||
python-multipart==0.0.22
|
||||
pytest==9.1.1
|
||||
rjsmin==1.2.5
|
||||
uvicorn==0.41.0
|
||||
|
|
|
|||
88
src/main.py
88
src/main.py
|
|
@ -19,7 +19,8 @@ from fastapi import FastAPI, Header, HTTPException, Path as PathParam, Query, Re
|
|||
from fastapi.exceptions import RequestValidationError
|
||||
from fastapi.responses import JSONResponse, RedirectResponse
|
||||
from fastapi.staticfiles import StaticFiles
|
||||
from pydantic import BaseModel, Field, PositiveInt, field_validator, model_validator
|
||||
from pydantic import BaseModel, Field, PositiveInt, ValidationError, field_validator, model_validator
|
||||
from starlette.datastructures import UploadFile
|
||||
|
||||
from src import dashboard_auth, gitea_proxy, passkeys
|
||||
from src.available_issue_snapshot_store import AvailableIssueSnapshotStore
|
||||
|
|
@ -359,6 +360,34 @@ class IssueComment(BaseModel):
|
|||
return value
|
||||
|
||||
|
||||
def _validate_attachment_metadata(filename: str, content_type: str) -> None:
|
||||
expected = {
|
||||
"image/png": {"png"},
|
||||
"image/jpeg": {"jpg", "jpeg"},
|
||||
"image/webp": {"webp"},
|
||||
}
|
||||
if content_type not in expected:
|
||||
raise ValueError("attachment content type must be PNG, JPEG, or WebP")
|
||||
if "/" in filename or "\\" in filename or any(ord(character) < 32 for character in filename):
|
||||
raise ValueError("attachment filename must be a plain file name")
|
||||
extension = filename.rsplit(".", 1)[-1].lower() if "." in filename else ""
|
||||
if extension not in expected[content_type]:
|
||||
raise ValueError("attachment filename must match the selected image type")
|
||||
|
||||
|
||||
def _validate_attachment_content(content_type: str, content: bytes) -> bytes:
|
||||
if len(content) > 2 * 1024 * 1024:
|
||||
raise ValueError("screenshot must be 2 MB or smaller")
|
||||
signatures = {
|
||||
"image/png": content.startswith(b"\x89PNG\r\n\x1a\n"),
|
||||
"image/jpeg": content.startswith(b"\xff\xd8\xff"),
|
||||
"image/webp": content.startswith(b"RIFF") and content[8:12] == b"WEBP",
|
||||
}
|
||||
if not signatures.get(content_type, False):
|
||||
raise ValueError("file contents do not match the selected image type")
|
||||
return content
|
||||
|
||||
|
||||
class IssueAttachment(BaseModel):
|
||||
filename: str = Field(min_length=1, max_length=255)
|
||||
content_type: Literal["image/png", "image/jpeg", "image/webp"]
|
||||
|
|
@ -366,20 +395,7 @@ class IssueAttachment(BaseModel):
|
|||
|
||||
@model_validator(mode="after")
|
||||
def validate_filename(self):
|
||||
if (
|
||||
"/" in self.filename
|
||||
or "\\" in self.filename
|
||||
or any(ord(character) < 32 for character in self.filename)
|
||||
):
|
||||
raise ValueError("attachment filename must be a plain file name")
|
||||
extension = self.filename.rsplit(".", 1)[-1].lower() if "." in self.filename else ""
|
||||
expected = {
|
||||
"image/png": {"png"},
|
||||
"image/jpeg": {"jpg", "jpeg"},
|
||||
"image/webp": {"webp"},
|
||||
}
|
||||
if extension not in expected[self.content_type]:
|
||||
raise ValueError("attachment filename must match the selected image type")
|
||||
_validate_attachment_metadata(self.filename, self.content_type)
|
||||
return self
|
||||
|
||||
def content(self) -> bytes:
|
||||
|
|
@ -387,16 +403,12 @@ class IssueAttachment(BaseModel):
|
|||
content = base64.b64decode(self.data, validate=True)
|
||||
except (ValueError, binascii.Error) as exc:
|
||||
raise ValueError("attachment data must be valid base64") from exc
|
||||
if len(content) > 2 * 1024 * 1024:
|
||||
raise ValueError("screenshot must be 2 MB or smaller")
|
||||
signatures = {
|
||||
"image/png": content.startswith(b"\x89PNG\r\n\x1a\n"),
|
||||
"image/jpeg": content.startswith(b"\xff\xd8\xff"),
|
||||
"image/webp": content.startswith(b"RIFF") and content[8:12] == b"WEBP",
|
||||
}
|
||||
if not signatures[self.content_type]:
|
||||
raise ValueError("file contents do not match the selected image type")
|
||||
return content
|
||||
return _validate_attachment_content(self.content_type, content)
|
||||
|
||||
|
||||
def _validate_binary_attachment(filename: str, content_type: str, content: bytes) -> bytes:
|
||||
_validate_attachment_metadata(filename, content_type)
|
||||
return _validate_attachment_content(content_type, content)
|
||||
|
||||
|
||||
class IssueCreation(BaseModel):
|
||||
|
|
@ -3107,7 +3119,7 @@ async def comment_on_assigned_issue(
|
|||
|
||||
@app.post("/api/v1/repos/{owner}/{repo}/issues/{number}/attachments", status_code=201)
|
||||
async def attach_to_assigned_issue(
|
||||
attachment: IssueAttachment,
|
||||
request: Request,
|
||||
owner: str,
|
||||
repo: str,
|
||||
number: int = PathParam(gt=0),
|
||||
|
|
@ -3115,15 +3127,27 @@ async def attach_to_assigned_issue(
|
|||
):
|
||||
repository = f"{owner}/{repo}"
|
||||
try:
|
||||
content = attachment.content()
|
||||
except ValueError as exc:
|
||||
if request.headers.get("content-type", "").lower().startswith("multipart/form-data"):
|
||||
form = await request.form()
|
||||
uploaded = form.get("file")
|
||||
if not isinstance(uploaded, UploadFile):
|
||||
raise ValueError("screenshot file is required")
|
||||
filename = str(uploaded.filename or "")
|
||||
content_type = str(uploaded.content_type or "")
|
||||
content = _validate_binary_attachment(filename, content_type, await uploaded.read())
|
||||
else:
|
||||
attachment = IssueAttachment.model_validate(await request.json())
|
||||
filename = attachment.filename
|
||||
content_type = attachment.content_type
|
||||
content = attachment.content()
|
||||
except (ValueError, ValidationError) as exc:
|
||||
raise HTTPException(status_code=422, detail=str(exc)) from exc
|
||||
async def upload_attachment():
|
||||
result = await gitea_proxy.upload_assigned_issue_attachment(
|
||||
repository,
|
||||
number,
|
||||
attachment.filename,
|
||||
attachment.content_type,
|
||||
filename,
|
||||
content_type,
|
||||
content,
|
||||
)
|
||||
safe_name = (
|
||||
|
|
@ -3142,8 +3166,8 @@ async def attach_to_assigned_issue(
|
|||
"issue-attachment",
|
||||
repository,
|
||||
number,
|
||||
attachment.filename,
|
||||
attachment.content_type,
|
||||
filename,
|
||||
content_type,
|
||||
hashlib.sha256(content).hexdigest(),
|
||||
),
|
||||
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
|
||||
|
|
|
|||
|
|
@ -5,7 +5,8 @@ from starlette.types import ASGIApp, Message, Receive, Scope, Send
|
|||
|
||||
SESSION_BODY_LIMIT = 16 * 1024
|
||||
API_MUTATION_BODY_LIMIT = 64 * 1024
|
||||
ISSUE_ATTACHMENT_BODY_LIMIT = 3 * 1024 * 1024
|
||||
ISSUE_ATTACHMENT_BODY_LIMIT = 2 * 1024 * 1024 + 64 * 1024
|
||||
LEGACY_JSON_ATTACHMENT_BODY_LIMIT = 3 * 1024 * 1024
|
||||
MUTATION_METHODS = frozenset({"POST", "PUT", "PATCH"})
|
||||
|
||||
|
||||
|
|
@ -50,6 +51,11 @@ class RequestBodyLimitMiddleware:
|
|||
if limit is None:
|
||||
await self.app(scope, receive, send)
|
||||
return
|
||||
if (
|
||||
limit == ISSUE_ATTACHMENT_BODY_LIMIT
|
||||
and self._header(scope, b"content-type").startswith(b"application/json")
|
||||
):
|
||||
limit = LEGACY_JSON_ATTACHMENT_BODY_LIMIT
|
||||
|
||||
declared_length = self._content_length(scope)
|
||||
if declared_length is not None and declared_length > limit:
|
||||
|
|
@ -77,6 +83,13 @@ class RequestBodyLimitMiddleware:
|
|||
|
||||
await self.app(scope, replay, send)
|
||||
|
||||
@staticmethod
|
||||
def _header(scope: Scope, wanted: bytes) -> bytes:
|
||||
for name, value in scope.get("headers", []):
|
||||
if name.lower() == wanted:
|
||||
return value.lower()
|
||||
return b""
|
||||
|
||||
@staticmethod
|
||||
def _content_length(scope: Scope) -> int | None:
|
||||
for name, value in scope.get("headers", []):
|
||||
|
|
|
|||
|
|
@ -498,7 +498,7 @@ async def test_mobile_dashboard_loads_and_operates_authored_message_outbox():
|
|||
assert "authoredOutbox.reconcileBackground(records)" in html
|
||||
|
||||
|
||||
def test_screenshot_comment_admits_bytes_to_indexeddb_without_putting_them_in_localstorage():
|
||||
def test_screenshot_comment_admits_blob_to_indexeddb_without_putting_bytes_in_localstorage():
|
||||
script = f"""
|
||||
const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});
|
||||
const values = new Map(); const mirrors=[];
|
||||
|
|
@ -508,12 +508,15 @@ const outbox=createAuthoredOutbox({{
|
|||
backgroundSync:{{reconcile:async(items,lane)=>mirrors.push({{items,lane}}),requestSync:async()=>{{}}}},
|
||||
}});
|
||||
(async()=>{{
|
||||
const blob=new Blob(['PRIVATE-IMAGE-BYTES'],{{type:'image/png'}});
|
||||
const admission=await outbox.enqueueDurably({{
|
||||
kind:'issue-comment',repository:'stackchain/dashboard',number:477,
|
||||
body:'Broken at 320px',operationId:'comment-image-477',
|
||||
attachment:{{filename:'phone.png',contentType:'image/png',data:'PRIVATE-IMAGE-BYTES'}},
|
||||
attachment:{{filename:'phone.png',contentType:'image/png',blob}},
|
||||
}});
|
||||
process.stdout.write(JSON.stringify({{admission,local:values.get('stackchain.authored-outbox.v1'),mirrored:mirrors[0]}}));
|
||||
const durable=mirrors[0].items[0].attachment;
|
||||
process.stdout.write(JSON.stringify({{admission,local:values.get('stackchain.authored-outbox.v1'),
|
||||
mirrored:{{lane:mirrors[0].lane,isBlob:durable.blob instanceof Blob,size:durable.blob?.size,text:await durable.blob?.text()}}}}));
|
||||
}})();
|
||||
"""
|
||||
output = run_node(script)
|
||||
|
|
@ -523,8 +526,9 @@ const outbox=createAuthoredOutbox({{
|
|||
assert local_item["attachment"] == {
|
||||
"filename": "phone.png", "contentType": "image/png", "stored": True
|
||||
}
|
||||
assert output["mirrored"]["lane"] == "authored"
|
||||
assert output["mirrored"]["items"][0]["attachment"]["data"] == "PRIVATE-IMAGE-BYTES"
|
||||
assert output["mirrored"] == {
|
||||
"lane": "authored", "isBlob": True, "size": 19, "text": "PRIVATE-IMAGE-BYTES"
|
||||
}
|
||||
assert output["admission"]["durability"] == "background"
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -82,7 +82,10 @@ const store = {{
|
|||
}};
|
||||
let uploadAttempts=0;
|
||||
const fetchJson=async(url,options={{}})=>{{
|
||||
state.calls.push({{url,key:options.headers?.['Idempotency-Key'],body:options.body&&JSON.parse(options.body)}});
|
||||
let body=null;
|
||||
if(options.body instanceof FormData){{const file=options.body.get('file');body={{filename:file.name,multipart:true}};}}
|
||||
else if(options.body)body=JSON.parse(options.body);
|
||||
state.calls.push({{url,key:options.headers?.['Idempotency-Key'],body}});
|
||||
if(url==='api/v1/background-identity')return{{login:'timmy'}};
|
||||
if(url.endsWith('/issues'))return{{repository:'stackchain/dashboard',number:469,title:'Broken mobile layout'}};
|
||||
if(url.endsWith('/attachments') && uploadAttempts++ === 0){{const error=new Error('Upload unavailable');error.status=503;throw error;}}
|
||||
|
|
@ -113,6 +116,37 @@ const fetchJson=async(url,options={{}})=>{{
|
|||
assert output["second"]["confirmed"][0]["number"] == 469
|
||||
|
||||
|
||||
def test_closed_app_sync_uploads_blob_as_multipart_and_preserves_original_bytes():
|
||||
script = f"""
|
||||
const createBackgroundIssueSync=require({json.dumps(str(SYNC))});
|
||||
const blob=new Blob(['offline-binary'],{{type:'image/png'}});
|
||||
const item={{id:'binary',operationId:'binary',ownerLogin:'timmy',repository:'o/r',title:'Visual',body:'',labelIds:[],
|
||||
attachment:{{filename:'phone.png',contentType:'image/png',blob}}}};
|
||||
let queued=true;
|
||||
const observed={{}};
|
||||
const store={{claimNext:async()=>queued?(queued=false,{{...item}}):null,update:async()=>{{}},complete:async()=>{{}},release:async()=>{{}},fail:async()=>{{}},countBlocked:async()=>0}};
|
||||
const fetchJson=async(url,options={{}})=>{{
|
||||
if(url==='api/v1/background-identity')return{{login:'timmy'}};
|
||||
if(url.endsWith('/issues'))return{{number:7}};
|
||||
if(url.endsWith('/attachments')){{const file=options.body.get('file');Object.assign(observed,{{
|
||||
isFormData:options.body instanceof FormData,contentTypeHeader:options.headers['Content-Type']||null,
|
||||
filename:file.name,type:file.type,size:file.size,text:await file.text()
|
||||
}});return{{markdown:''}};}}
|
||||
return{{id:8}};
|
||||
}};
|
||||
(async()=>{{await createBackgroundIssueSync({{store,fetchJson}}).flush();process.stdout.write(JSON.stringify(observed));}})();
|
||||
"""
|
||||
|
||||
assert run_node(script) == {
|
||||
"isFormData": True,
|
||||
"contentTypeHeader": None,
|
||||
"filename": "phone.png",
|
||||
"type": "image/png",
|
||||
"size": 14,
|
||||
"text": "offline-binary",
|
||||
}
|
||||
|
||||
|
||||
def test_capture_attachment_stage_keys_remain_within_idempotency_limit():
|
||||
script = f"""
|
||||
const createBackgroundIssueSync=require({json.dumps(str(SYNC))});
|
||||
|
|
@ -175,7 +209,10 @@ const store={{claimNext:async()=>item?{{...item}}:null,update:async(_id,transfor
|
|||
fail:async()=>{{}},countBlocked:async()=>0}};
|
||||
const fetchJson=async(url,options={{}})=>{{
|
||||
if(url==='api/v1/background-identity')return{{login:'timmy'}};
|
||||
state.calls.push({{url,key:options.headers?.['Idempotency-Key'],body:options.body&&JSON.parse(options.body)}});
|
||||
let body=null;
|
||||
if(options.body instanceof FormData){{const file=options.body.get('file');body={{filename:file.name,multipart:true}};}}
|
||||
else if(options.body)body=JSON.parse(options.body);
|
||||
state.calls.push({{url,key:options.headers?.['Idempotency-Key'],body}});
|
||||
if(url.endsWith('/attachments'))return{{markdown:''}};
|
||||
if(commentAttempts++===0){{const error=new Error('Comment unavailable');error.status=503;throw error;}}
|
||||
return{{id:91}};
|
||||
|
|
@ -435,7 +472,7 @@ const transaction=work=>{{const run=tail.then(()=>work({{
|
|||
]
|
||||
|
||||
|
||||
def test_reconciling_a_lightweight_attachment_reference_preserves_indexeddb_bytes():
|
||||
def test_reconciling_a_lightweight_attachment_reference_preserves_indexeddb_blob():
|
||||
script = f"""
|
||||
const createBackgroundIssueSync = require({json.dumps(str(SYNC))});
|
||||
const records=new Map();let tail=Promise.resolve();
|
||||
|
|
@ -447,21 +484,23 @@ const transaction=work=>{{const run=tail.then(()=>work({{
|
|||
const store=createBackgroundIssueSync.createIssueSyncStore({{transaction}});
|
||||
await store.reconcile([{{
|
||||
id:'first',operationId:'first',ownerLogin:'timmy',status:'queued',
|
||||
attachment:{{filename:'one.png',contentType:'image/png',data:'first-image-bytes'}},
|
||||
attachment:{{filename:'one.png',contentType:'image/png',blob:new Blob(['first-image-bytes'],{{type:'image/png'}})}},
|
||||
}}]);
|
||||
await store.reconcile([
|
||||
{{id:'first',operationId:'first',ownerLogin:'timmy',status:'queued',
|
||||
attachment:{{filename:'one.png',contentType:'image/png',stored:true}}}},
|
||||
{{id:'second',operationId:'second',ownerLogin:'timmy',status:'queued',
|
||||
attachment:{{filename:'two.png',contentType:'image/png',data:'second-image-bytes'}}}},
|
||||
attachment:{{filename:'two.png',contentType:'image/png',data:'c2Vjb25kLWltYWdlLWJ5dGVz'}}}},
|
||||
]);
|
||||
process.stdout.write(JSON.stringify(await store.snapshot()));
|
||||
const snapshot=await store.snapshot();
|
||||
process.stdout.write(JSON.stringify({{first:{{isBlob:snapshot[0].attachment.blob instanceof Blob,
|
||||
text:await snapshot[0].attachment.blob?.text()}},second:snapshot[1].attachment.data}}));
|
||||
}})();
|
||||
"""
|
||||
output = run_node(script)
|
||||
|
||||
assert output[0]["attachment"]["data"] == "first-image-bytes"
|
||||
assert output[1]["attachment"]["data"] == "second-image-bytes"
|
||||
assert output["first"] == {"isBlob": True, "text": "first-image-bytes"}
|
||||
assert output["second"] == "c2Vjb25kLWltYWdlLWJ5dGVz"
|
||||
|
||||
|
||||
def test_issue_sync_store_hydrates_one_capture_by_key_without_scanning_all_records():
|
||||
|
|
|
|||
|
|
@ -18,15 +18,19 @@ def run_node(script: str) -> str:
|
|||
).stdout
|
||||
|
||||
|
||||
def test_mobile_attachment_prepares_comment_and_reuses_confirmed_upload():
|
||||
def test_mobile_attachment_prepares_comment_and_reuses_confirmed_binary_upload():
|
||||
script = f"""
|
||||
const attachment = require({json.dumps(str(ATTACHMENT))});
|
||||
const calls = [];
|
||||
const file = {{name:'checkout.png', type:'image/png', size:8}};
|
||||
const file = new Blob(['png-bytes'],{{type:'image/png'}}); file.name='checkout.png';
|
||||
const controller = attachment.create({{
|
||||
createOperationId: () => 'attachment-comment-471',
|
||||
readDataUrl: async selected => {{ calls.push('read:' + selected.name); return 'data:image/png;base64,iVBORw0KGgo='; }},
|
||||
upload: async payload => {{ calls.push(payload); return {{markdown:''}}; }},
|
||||
readDataUrl: async () => {{ throw new Error('base64 conversion must not run'); }},
|
||||
upload: async payload => {{ calls.push({{
|
||||
repository:payload.repository,number:payload.number,filename:payload.filename,
|
||||
content_type:payload.content_type,isBlob:payload.blob instanceof Blob,
|
||||
text:await payload.blob.text(),operation_id:payload.operation_id
|
||||
}}); return {{markdown:''}}; }},
|
||||
}});
|
||||
controller.select(file);
|
||||
(async()=>{{
|
||||
|
|
@ -40,19 +44,17 @@ controller.select(file);
|
|||
"Layout breaks\n\n"
|
||||
)
|
||||
assert output["second"] == output["first"]
|
||||
assert output["calls"] == [
|
||||
"read:checkout.png",
|
||||
{
|
||||
"repository": "stackchain/api",
|
||||
"number": 17,
|
||||
"filename": "checkout.png",
|
||||
"content_type": "image/png",
|
||||
"data": "iVBORw0KGgo=",
|
||||
"operation_id": "attachment-comment-471",
|
||||
},
|
||||
]
|
||||
assert output["calls"] == [{
|
||||
"repository": "stackchain/api",
|
||||
"number": 17,
|
||||
"filename": "checkout.png",
|
||||
"content_type": "image/png",
|
||||
"isBlob": True,
|
||||
"text": "png-bytes",
|
||||
"operation_id": "attachment-comment-471",
|
||||
}]
|
||||
assert output["state"] == {
|
||||
"name": "checkout.png", "size": 8, "uploaded": True
|
||||
"name": "checkout.png", "size": 9, "uploaded": True
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -82,26 +84,50 @@ controller.select({{name:'first.png',type:'image/png',size:8}});
|
|||
]
|
||||
|
||||
|
||||
def test_selected_screenshot_serializes_for_durable_issue_capture_without_uploading():
|
||||
def test_selected_screenshot_serializes_as_binary_blob_without_base64_expansion():
|
||||
script = f"""
|
||||
const attachment = require({json.dumps(str(ATTACHMENT))});
|
||||
const calls=[];
|
||||
const file=new Blob(['binary-image'],{{type:'image/webp'}});
|
||||
file.name='phone.webp';
|
||||
const controller=attachment.create({{
|
||||
readDataUrl:async file=>{{calls.push('read:'+file.name);return 'data:image/webp;base64,UklGRg==';}},
|
||||
readDataUrl:async()=>{{throw new Error('base64 conversion must not run');}},
|
||||
upload:async()=>{{calls.push('upload');}},
|
||||
}});
|
||||
controller.select({{name:'phone.webp',type:'image/webp',size:8}});
|
||||
controller.serialize().then(value=>process.stdout.write(JSON.stringify({{value,calls}})));
|
||||
controller.select(file);
|
||||
controller.serialize().then(async value=>process.stdout.write(JSON.stringify({{
|
||||
filename:value.filename,contentType:value.contentType,
|
||||
isBlob:value.blob instanceof Blob,size:value.blob.size,text:await value.blob.text(),calls
|
||||
}})));
|
||||
"""
|
||||
output = json.loads(run_node(script))
|
||||
|
||||
assert output == {
|
||||
"value": {
|
||||
"filename": "phone.webp",
|
||||
"contentType": "image/webp",
|
||||
"data": "UklGRg==",
|
||||
},
|
||||
"calls": ["read:phone.webp"],
|
||||
"filename": "phone.webp",
|
||||
"contentType": "image/webp",
|
||||
"isBlob": True,
|
||||
"size": 12,
|
||||
"text": "binary-image",
|
||||
"calls": [],
|
||||
}
|
||||
|
||||
|
||||
def test_binary_screenshot_builds_multipart_body_with_original_bytes():
|
||||
script = f"""
|
||||
const attachment=require({json.dumps(str(ATTACHMENT))});
|
||||
const blob=new Blob(['original-bytes'],{{type:'image/png'}});
|
||||
const form=attachment.multipart({{filename:'phone.png',contentType:'image/png',blob}});
|
||||
const file=form.get('file');
|
||||
(async()=>process.stdout.write(JSON.stringify({{
|
||||
filename:file.name,type:file.type,size:file.size,text:await file.text()
|
||||
}})))();
|
||||
"""
|
||||
|
||||
assert json.loads(run_node(script)) == {
|
||||
"filename": "phone.png",
|
||||
"type": "image/png",
|
||||
"size": 14,
|
||||
"text": "original-bytes",
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -220,7 +246,7 @@ process.stdout.write(JSON.stringify({{invalid,selected,removed:{{hidden:preview.
|
|||
}
|
||||
|
||||
|
||||
def test_issue_comment_actions_upload_before_posting_and_clear_after_acceptance():
|
||||
def test_issue_comment_actions_upload_binary_multipart_before_posting_and_clear_after_acceptance():
|
||||
source = DASHBOARD.read_text()
|
||||
|
||||
assert "issueAttachment.mount({" in source
|
||||
|
|
@ -229,6 +255,7 @@ def test_issue_comment_actions_upload_before_posting_and_clear_after_acceptance(
|
|||
assert "issueAttachmentController.prepareComment(selectedIssue, body)" in source
|
||||
assert source.count("issueAttachmentController.clear();") >= 2
|
||||
assert "'Idempotency-Key': payload.operation_id" in source
|
||||
assert "body: issueAttachment.multipart(payload)" in source
|
||||
|
||||
|
||||
def test_closing_issue_sheet_cannot_carry_a_screenshot_to_another_issue():
|
||||
|
|
|
|||
|
|
@ -58,6 +58,32 @@ async def test_attachment_endpoint_uploads_valid_screenshot_to_assigned_issue(mo
|
|||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_attachment_endpoint_accepts_binary_multipart_without_base64_expansion(monkeypatch):
|
||||
calls = []
|
||||
|
||||
async def upload(repository, number, filename, content_type, content):
|
||||
calls.append((repository, number, filename, content_type, content))
|
||||
return {
|
||||
"name": filename,
|
||||
"url": "https://forge.example/attachments/checkout.png",
|
||||
"size": len(content),
|
||||
}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "upload_assigned_issue_attachment", upload)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.post(
|
||||
"/api/v1/repos/stackchain/api/issues/17/attachments",
|
||||
files={"file": ("checkout.png", PNG_BYTES, "image/png")},
|
||||
)
|
||||
|
||||
assert response.status_code == 201
|
||||
assert calls == [
|
||||
("stackchain/api", 17, "checkout.png", "image/png", PNG_BYTES)
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_attachment_endpoint_replays_confirmed_upload_for_same_key(monkeypatch):
|
||||
calls = []
|
||||
|
|
|
|||
|
|
@ -234,20 +234,27 @@ const calls=[];let uploadAttempts=0;
|
|||
const outbox=createIssueOutbox({{
|
||||
storage,getOwnerLogin:()=>'timmy',createOperationId:()=>'image-op',
|
||||
fetchJson:async(url,options={{}})=>{{
|
||||
calls.push({{url,key:options.headers?.['Idempotency-Key'],body:options.body&&JSON.parse(options.body)}});
|
||||
const form=options.body instanceof FormData ? options.body : null;
|
||||
const file=form?.get('file');
|
||||
calls.push({{url,key:options.headers?.['Idempotency-Key'],multipart:Boolean(form),
|
||||
filename:file?.name,text:file?await file.text():null}});
|
||||
if(url.endsWith('/issues'))return{{repository:'o/r',number:7,title:'Visual bug'}};
|
||||
if(url.endsWith('/attachments') && uploadAttempts++ === 0){{const error=new Error('offline');error.status=503;throw error;}}
|
||||
if(url.endsWith('/attachments'))return{{markdown:''}};
|
||||
return{{id:8}};
|
||||
}},
|
||||
}});
|
||||
const queued=outbox.enqueue({{repository:'o/r',title:'Visual bug',attachment:{{filename:'screen.png',contentType:'image/png',data:'abc'}}}});
|
||||
const queued=outbox.enqueue({{repository:'o/r',title:'Visual bug',attachment:{{filename:'screen.png',contentType:'image/png',data:'b2ZmbGluZQ=='}}}});
|
||||
(async()=>{{await outbox.flush('timmy');const partial=outbox.list()[0];const result=await outbox.retry(queued.id,'timmy');process.stdout.write(JSON.stringify({{calls,partial,result,remaining:outbox.list()}}));}})();
|
||||
"""
|
||||
output = run_node(script)
|
||||
|
||||
assert output["partial"]["deliveredIssue"]["number"] == 7
|
||||
assert [call["url"] for call in output["calls"]].count("api/v1/repos/o/r/issues") == 1
|
||||
uploads = [call for call in output["calls"] if call["url"].endswith("/attachments")]
|
||||
assert uploads[-1]["multipart"] is True
|
||||
assert uploads[-1]["filename"] == "screen.png"
|
||||
assert uploads[-1]["text"] == "offline"
|
||||
assert output["calls"][-1]["url"] == "api/v1/repos/o/r/issues/7/comments"
|
||||
assert output["result"]["confirmed"][0]["number"] == 7
|
||||
assert output["remaining"] == []
|
||||
|
|
@ -452,14 +459,14 @@ Promise.resolve().then(async () => {{
|
|||
assert output["items"][0]["title"] == "Keep this"
|
||||
|
||||
|
||||
def test_durable_screenshot_is_mirrored_before_a_payload_free_local_index_is_committed():
|
||||
def test_durable_screenshot_is_mirrored_as_blob_before_a_payload_free_local_index_is_committed():
|
||||
script = f"""
|
||||
const createIssueOutbox = require({json.dumps(str(OUTBOX))});
|
||||
const values = new Map(); const events = []; const snapshots = [];
|
||||
const storage = {{
|
||||
getItem:key => values.get(key) || null,
|
||||
setItem:(key,value) => {{
|
||||
if (value.includes('base64-screenshot-bytes')) throw new Error('screenshot leaked into localStorage');
|
||||
if (value.includes('binary-screenshot-bytes')) throw new Error('screenshot leaked into localStorage');
|
||||
events.push('local-index'); values.set(key,value);
|
||||
}},
|
||||
}};
|
||||
|
|
@ -471,26 +478,35 @@ const outbox = createIssueOutbox({{
|
|||
}},
|
||||
}});
|
||||
(async () => {{
|
||||
const blob=new Blob(['binary-screenshot-bytes'],{{type:'image/webp'}});
|
||||
const result = await outbox.enqueueDurably({{
|
||||
repository:'stackchain/dashboard', title:'Mobile layout',
|
||||
attachment:{{filename:'phone.webp',contentType:'image/webp',data:'base64-screenshot-bytes'}},
|
||||
attachment:{{filename:'phone.webp',contentType:'image/webp',blob}},
|
||||
}});
|
||||
const durable=snapshots[0][0].attachment;
|
||||
process.stdout.write(JSON.stringify({{
|
||||
events, mirrored:snapshots[0][0], local:outbox.list()[0], result:result.item,
|
||||
raw:values.get('stackchain.issue-outbox.v1'),
|
||||
events, mirrored:{{filename:durable.filename,contentType:durable.contentType,
|
||||
isBlob:durable.blob instanceof Blob,size:durable.blob?.size,text:await durable.blob?.text()}},
|
||||
local:outbox.list()[0], result:result.item, raw:values.get('stackchain.issue-outbox.v1'),
|
||||
}}));
|
||||
}})();
|
||||
"""
|
||||
output = run_node(script)
|
||||
|
||||
assert output["events"] == ["indexeddb", "local-index", "sync"]
|
||||
assert output["mirrored"]["attachment"]["data"] == "base64-screenshot-bytes"
|
||||
assert output["mirrored"] == {
|
||||
"filename": "phone.webp",
|
||||
"contentType": "image/webp",
|
||||
"isBlob": True,
|
||||
"size": 23,
|
||||
"text": "binary-screenshot-bytes",
|
||||
}
|
||||
assert output["local"]["attachment"] == {
|
||||
"filename": "phone.webp",
|
||||
"contentType": "image/webp",
|
||||
"stored": True,
|
||||
}
|
||||
assert "base64-screenshot-bytes" not in output["raw"]
|
||||
assert "binary-screenshot-bytes" not in output["raw"]
|
||||
assert output["result"]["attachment"] == output["local"]["attachment"]
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -116,7 +116,7 @@ def test_request_limits_are_route_specific_and_cover_api_mutations():
|
|||
main.request_body_limit(
|
||||
"POST", "/api/v1/repos/stackchain/project/issues/17/attachments"
|
||||
)
|
||||
== 3 * 1024 * 1024
|
||||
== 2 * 1024 * 1024 + 64 * 1024
|
||||
)
|
||||
assert main.request_body_limit("POST", "/unrelated/attachments") is None
|
||||
assert main.request_body_limit("POST", "/unrelated") is None
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user