[go: up one dir, main page]
Skip to content

Assign content-type when returning generic package file

Sylvia Shenrequested to merge
444768-keep-generic-content-type into master

What does this MR do and why?

Currently, when customer uploads a generic package file, the system will store the file in object storage as binary/octet-stream in MinIO and when customers tries to (for example) get a css file via a html page, because of the wrong content type, the browser will stop the file from being downloaded.

What are the possible options?

  1. Setup correct content type while uploading the file by filename

    Carrierwave now will detect content type directly, after checking the source code, we can assign a different content type with the callback by the setter. However, when the upload method is direct upload, we will use object storage concern and the file is actually a UploadedFile in cache stage, because of that, we couldn't add before callback for store, instead, we need to add before callback for cache. However, because our implementation of ObjectStorage::Concern#cache!, we will return directly without triggering the callback, and even if we add the callback back, since we don't have the model#file_name and real uploaded file's name, we couldn't detect the correct content type even if we revise UploadedFile and ObjectStorage#cache!, more changes will be required.

Because of the scope of this issue, we decided to go for option 2, which is done in this PR:

  1. Detect content type when customer get the package file

    • We add a new method Gitlab::Utils::MimeType#from_filename, and move the logic in SendFileUpload to it to unify the logic of detecting content type.
    • We add a new feature flag packages_generic_package_content_type to control the rollout since this change will affect the whole generic package.

References

Screenshots or screen recordings

N/A

How to set up and validate locally

1. Create test files

body {
    background-color: #f0f0f0;
    font-family: Arial, sans-serif;
    color: #333;
}
.test-element {
    background-color: #007bff;
    color: white;
    padding: 10px;
    border-radius: 5px;
}
  1. Create a test HTML file
<!DOCTYPE html>
<html>
<head>
    <title>CSS Content-Type Test</title>
    <link rel="stylesheet" href="http://127.0.0.1:3000/api/v4/projects/YOUR_PROJECT_ID/packages/generic/css-test-package/1.0.0/test-styles.css">
</head>
<body>
    <h1>CSS Content-Type Test</h1>
    <div class="test-element">This should be styled if CSS loads correctly</div>
    <p>Check browser console for any MIME type errors.</p>
</body>
</html>
  1. Upload CSS file to Package Registry
  2. Test BEFORE enabling feature flag by opening the html file, CSS fails to load
  3. AFTER enabling feature flag, reload the page, CSS loads successfully
gdk rails console

# In Rails Console
Feature.enable(:packages_generic_package_content_type)

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #444768

Edited by Sylvia Shen

Merge request reports