Reapply "feat(tus): split into separate creation and upload requests"

This reverts commit 4f3fa6d.

Author: jpnurmi

src/transports/sentry_http_transport.c

@@ -225,10 +225,6 @@ prepare_tus_request_common(
     h->key = "x-sentry-auth";
     h->value = sentry__dsn_get_auth_header(dsn, user_agent);
 
-    h = &req->headers[req->headers_len++];
-    h->key = "content-type";
-    h->value = sentry__string_clone(TUS_MIME);
-
     h = &req->headers[req->headers_len++];
     h->key = "tus-resumable";
     h->value = sentry__string_clone("1.0.0");
@@ -241,18 +237,46 @@ prepare_tus_request_common(
 }
 
 static sentry_prepared_http_request_t *
-prepare_tus_request(const sentry_path_t *path, size_t file_size,
-    const sentry_dsn_t *dsn, const char *user_agent)
+prepare_tus_upload_request(
+    const char *location, const sentry_path_t *path, size_t file_size)
 {
-    if (!path) {
+    if (!location || !path) {
         return NULL;
     }
+
     sentry_prepared_http_request_t *req
-        = prepare_tus_request_common(file_size, dsn, user_agent);
-    if (req) {
-        req->body_path = sentry__path_clone(path);
-        req->body_len = file_size;
+        = SENTRY_MAKE(sentry_prepared_http_request_t);
+    if (!req) {
+        return NULL;
+    }
+    memset(req, 0, sizeof(*req));
+
+    req->headers = sentry_malloc(
+        sizeof(sentry_prepared_http_header_t) * TUS_MAX_HTTP_HEADERS);
+    if (!req->headers) {
+        sentry_free(req);
+        return NULL;
     }
+    req->headers_len = 0;
+
+    req->method = "PATCH";
+    req->url = sentry__string_clone(location);
+    req->body_path = sentry__path_clone(path);
+    req->body_len = file_size;
+
+    sentry_prepared_http_header_t *h;
+    h = &req->headers[req->headers_len++];
+    h->key = "tus-resumable";
+    h->value = sentry__string_clone("1.0.0");
+
+    h = &req->headers[req->headers_len++];
+    h->key = "content-type";
+    h->value = sentry__string_clone(TUS_MIME);
+
+    h = &req->headers[req->headers_len++];
+    h->key = "upload-offset";
+    h->value = sentry__string_clone("0");
+
     return req;
 }
 
@@ -307,8 +331,9 @@ tus_upload_ref(http_transport_state_t *state, const sentry_path_t *att_dir,
         return false;
     }
 
-    sentry_prepared_http_request_t *req = prepare_tus_request(
-        att_file, file_size, state->dsn, state->user_agent);
+    // Step 1: TUS creation request (POST, no body)
+    sentry_prepared_http_request_t *req
+        = prepare_tus_request_common(file_size, state->dsn, state->user_agent);
     if (!req) {
         sentry__path_free(att_file);
         return false;
@@ -339,19 +364,42 @@ tus_upload_ref(http_transport_state_t *state, const sentry_path_t *att_dir,
         return false;
     }
 
+    // Step 2: TUS upload request (PATCH with file body)
+    req = prepare_tus_upload_request(resp.location, att_file, file_size);
+    char *location = resp.location;
+    resp.location = NULL;
+    http_response_cleanup(&resp);
+
+    if (!req) {
+        sentry__path_free(att_file);
+        sentry_free(location);
+        return false;
+    }
+
+    memset(&resp, 0, sizeof(resp));
+    ok = state->send_func(state->client, req, &resp);
+    sentry__prepared_http_request_free(req);
+    http_response_cleanup(&resp);
+
+    if (!ok || resp.status_code != 204) {
+        sentry__path_free(att_file);
+        sentry_free(location);
+        return false;
+    }
+
     const char *ref_ct = sentry_value_as_string(
         sentry_value_get_by_key(entry, "content_type"));
     const char *att_type = sentry_value_as_string(
         sentry_value_get_by_key(entry, "attachment_type"));
     sentry_value_t att_length
         = sentry_value_get_by_key(entry, "attachment_length");
     sentry_value_incref(att_length);
-    sentry__envelope_add_attachment_ref(tus_envelope, resp.location, filename,
+    sentry__envelope_add_attachment_ref(tus_envelope, location, filename,
         *ref_ct ? ref_ct : NULL, *att_type ? att_type : NULL, att_length);
 
     sentry__path_remove(att_file);
     sentry__path_free(att_file);
-    http_response_cleanup(&resp);
+    sentry_free(location);
     return true;
 }
 
@@ -772,9 +820,16 @@ sentry__http_transport_get_bgworker(sentry_transport_t *transport)
 }
 
 sentry_prepared_http_request_t *
-sentry__prepare_tus_request(const sentry_path_t *path, size_t file_size,
-    const sentry_dsn_t *dsn, const char *user_agent)
+sentry__prepare_tus_create_request(
+    size_t file_size, const sentry_dsn_t *dsn, const char *user_agent)
+{
+    return prepare_tus_request_common(file_size, dsn, user_agent);
+}
+
+sentry_prepared_http_request_t *
+sentry__prepare_tus_upload_request(
+    const char *location, const sentry_path_t *path, size_t file_size)
 {
-    return prepare_tus_request(path, file_size, dsn, user_agent);
+    return prepare_tus_upload_request(location, path, file_size);
 }
 #endif

src/transports/sentry_http_transport.h

@@ -55,9 +55,10 @@ void sentry__http_transport_set_shutdown_client(
 
 #ifdef SENTRY_UNITTEST
 void *sentry__http_transport_get_bgworker(sentry_transport_t *transport);
-sentry_prepared_http_request_t *sentry__prepare_tus_request(
-    const sentry_path_t *path, size_t file_size, const sentry_dsn_t *dsn,
-    const char *user_agent);
+sentry_prepared_http_request_t *sentry__prepare_tus_create_request(
+    size_t file_size, const sentry_dsn_t *dsn, const char *user_agent);
+sentry_prepared_http_request_t *sentry__prepare_tus_upload_request(
+    const char *location, const sentry_path_t *path, size_t file_size);
 #endif
 
 #endif

tests/test_integration_tus.py

@@ -26,8 +26,11 @@ def test_tus_upload_large_attachment(cmake, httpserver):
         {"SENTRY_BACKEND": "none", "SENTRY_TRANSPORT_COMPRESSION": "Off"},
     )
 
-    location = "/api/123456/upload/abc123def456789/?length=104857600&signature=xyz"
+    upload_uri = "/api/123456/upload/abc123def456789/"
+    upload_qs = "length=104857600&signature=xyz"
+    location = httpserver.url_for(upload_uri) + "?" + upload_qs
 
+    # TUS creation request (POST, no body) -> 201 + Location
     httpserver.expect_oneshot_request(
         "/api/123456/upload/",
         headers={"tus-resumable": "1.0.0"},
@@ -37,6 +40,14 @@ def test_tus_upload_large_attachment(cmake, httpserver):
         headers={"Location": location},
     )
 
+    # TUS upload request (PATCH with body) -> 204
+    httpserver.expect_oneshot_request(
+        upload_uri,
+        method="PATCH",
+        headers={"tus-resumable": "1.0.0"},
+        query_string=upload_qs,
+    ).respond_with_data("", status=204)
+
     httpserver.expect_request(
         "/api/123456/envelope/",
         headers={"x-sentry-auth": auth_header},
@@ -51,27 +62,35 @@ def test_tus_upload_large_attachment(cmake, httpserver):
         env=env,
     )
 
-    assert len(httpserver.log) == 3
+    assert len(httpserver.log) == 4
 
-    # Find the upload request and envelope requests
+    # Find the create, upload, and envelope requests
+    create_req = None
     upload_req = None
     envelope_reqs = []
     for entry in httpserver.log:
         req = entry[0]
-        if "/upload/" in req.path:
+        if req.path == "/api/123456/upload/" and req.method == "POST":
+            create_req = req
+        elif upload_uri in req.path and req.method == "PATCH":
             upload_req = req
         elif "/envelope/" in req.path:
             envelope_reqs.append(req)
 
+    assert create_req is not None
     assert upload_req is not None
     assert len(envelope_reqs) == 2
 
+    # Verify TUS creation request headers
+    assert create_req.headers.get("tus-resumable") == "1.0.0"
+    upload_length = create_req.headers.get("upload-length")
+    assert upload_length is not None
+    assert int(upload_length) == 100 * 1024 * 1024
+
     # Verify TUS upload request headers
     assert upload_req.headers.get("tus-resumable") == "1.0.0"
     assert upload_req.headers.get("content-type") == "application/offset+octet-stream"
-    upload_length = upload_req.headers.get("upload-length")
-    assert upload_length is not None
-    assert int(upload_length) == 100 * 1024 * 1024
+    assert upload_req.headers.get("upload-offset") == "0"
 
     # One envelope has the resolved attachment-refs, the other is the original
     attachment_ref = None
@@ -183,7 +202,9 @@ def test_tus_crash_restart(cmake, httpserver):
     att_size = os.path.getsize(os.path.join(att_dir, att_files[0]))
     assert att_size >= 100 * 1024 * 1024
 
-    location = "/api/123456/upload/abc123def456789/?length=104857600&signature=xyz"
+    upload_uri = "/api/123456/upload/abc123def456789/"
+    upload_qs = "length=104857600&signature=xyz"
+    location = httpserver.url_for(upload_uri) + "?" + upload_qs
 
     # Second run: restart picks up crash and uploads via TUS
     httpserver.expect_oneshot_request(
@@ -195,6 +216,13 @@ def test_tus_crash_restart(cmake, httpserver):
         headers={"Location": location},
     )
 
+    httpserver.expect_oneshot_request(
+        upload_uri,
+        method="PATCH",
+        headers={"tus-resumable": "1.0.0"},
+        query_string=upload_qs,
+    ).respond_with_data("", status=204)
+
     httpserver.expect_request(
         "/api/123456/envelope/",
         headers={"x-sentry-auth": auth_header},
@@ -207,24 +235,32 @@ def test_tus_crash_restart(cmake, httpserver):
         env=env,
     )
 
-    assert len(httpserver.log) == 3
+    assert len(httpserver.log) == 4
 
+    create_req = None
     upload_req = None
     envelope_reqs = []
     for entry in httpserver.log:
         req = entry[0]
-        if "/upload/" in req.path:
+        if req.path == "/api/123456/upload/" and req.method == "POST":
+            create_req = req
+        elif upload_uri in req.path and req.method == "PATCH":
             upload_req = req
         elif "/envelope/" in req.path:
             envelope_reqs.append(req)
 
+    assert create_req is not None
     assert upload_req is not None
     assert len(envelope_reqs) == 2
 
+    # Verify TUS creation request headers
+    assert create_req.headers.get("tus-resumable") == "1.0.0"
+    assert int(create_req.headers.get("upload-length")) == 100 * 1024 * 1024
+
     # Verify TUS upload request headers
     assert upload_req.headers.get("tus-resumable") == "1.0.0"
     assert upload_req.headers.get("content-type") == "application/offset+octet-stream"
-    assert int(upload_req.headers.get("upload-length")) == 100 * 1024 * 1024
+    assert upload_req.headers.get("upload-offset") == "0"
 
     # One envelope has the resolved attachment-refs, the other is the original
     attachment_ref = None

tests/unit/test_envelopes.c

@@ -742,19 +742,15 @@ SENTRY_TEST(tus_request_preparation)
 {
     SENTRY_TEST_DSN_NEW_DEFAULT(dsn);
 
-    const char *test_file_str = SENTRY_TEST_PATH_PREFIX "sentry_test_tus_file";
-    sentry_path_t *test_file_path = sentry__path_from_str(test_file_str);
-    TEST_CHECK_INT_EQUAL(
-        sentry__path_write_buffer(test_file_path, "test-data", 9), 0);
-
+    // Test creation request (POST, no body)
     sentry_prepared_http_request_t *req
-        = sentry__prepare_tus_request(test_file_path, 9, dsn, NULL);
+        = sentry__prepare_tus_create_request(9, dsn, NULL);
     TEST_CHECK(!!req);
     TEST_CHECK_STRING_EQUAL(req->method, "POST");
     TEST_CHECK_STRING_EQUAL(
         req->url, "https://sentry.invalid:443/api/42/upload/");
-    TEST_CHECK(!!req->body_path);
-    TEST_CHECK_INT_EQUAL(req->body_len, 9);
+    TEST_CHECK(!req->body_path);
+    TEST_CHECK_INT_EQUAL(req->body_len, 0);
     TEST_CHECK(!req->body);
 
     bool has_tus_resumable = false;
@@ -769,15 +765,52 @@ SENTRY_TEST(tus_request_preparation)
             TEST_CHECK_STRING_EQUAL(req->headers[i].value, "9");
             has_upload_length = true;
         }
+        if (strcmp(req->headers[i].key, "content-type") == 0) {
+            has_content_type = true;
+        }
+    }
+    TEST_CHECK(has_tus_resumable);
+    TEST_CHECK(has_upload_length);
+    TEST_CHECK(!has_content_type);
+
+    sentry__prepared_http_request_free(req);
+
+    // Test upload request (PATCH with body)
+    const char *test_file_str = SENTRY_TEST_PATH_PREFIX "sentry_test_tus_file";
+    sentry_path_t *test_file_path = sentry__path_from_str(test_file_str);
+    TEST_CHECK_INT_EQUAL(
+        sentry__path_write_buffer(test_file_path, "test-data", 9), 0);
+
+    const char *location = "https://sentry.invalid/api/42/upload/abc123/";
+    req = sentry__prepare_tus_upload_request(location, test_file_path, 9);
+    TEST_CHECK(!!req);
+    TEST_CHECK_STRING_EQUAL(req->method, "PATCH");
+    TEST_CHECK_STRING_EQUAL(req->url, location);
+    TEST_CHECK(!!req->body_path);
+    TEST_CHECK_INT_EQUAL(req->body_len, 9);
+    TEST_CHECK(!req->body);
+
+    has_tus_resumable = false;
+    has_content_type = false;
+    bool has_upload_offset = false;
+    for (size_t i = 0; i < req->headers_len; i++) {
+        if (strcmp(req->headers[i].key, "tus-resumable") == 0) {
+            TEST_CHECK_STRING_EQUAL(req->headers[i].value, "1.0.0");
+            has_tus_resumable = true;
+        }
         if (strcmp(req->headers[i].key, "content-type") == 0) {
             TEST_CHECK_STRING_EQUAL(
                 req->headers[i].value, "application/offset+octet-stream");
             has_content_type = true;
         }
+        if (strcmp(req->headers[i].key, "upload-offset") == 0) {
+            TEST_CHECK_STRING_EQUAL(req->headers[i].value, "0");
+            has_upload_offset = true;
+        }
     }
     TEST_CHECK(has_tus_resumable);
-    TEST_CHECK(has_upload_length);
     TEST_CHECK(has_content_type);
+    TEST_CHECK(has_upload_offset);
 
     sentry__prepared_http_request_free(req);
     sentry__path_remove(test_file_path);