Uploaded images become inaccessible after the domain was changed.

[Octobug]

Describe the bug

Uploaded images become inaccessible after the domain was changed. I only tested it in development env.

To Reproduce

Steps to reproduce the behavior:

1. Under domain.old, you uploaded your custom avatar.
2. You updated domain.old to domain.new, and the uploaded avatar became inaccessible.
   • Domain changes are very common.

Expected behavior

Uploaded files should serve well no matter what the domain is.

Screenshots

Platform

• Device: Desktop
• OS: macOS Ventura 13.6.1
• Browser and version: Chrome 122.0.6261.129 (Official Build) (x86_64)
• Version: v1.2.5

[LinkinStars]

Thanks for the feedback. It seems that the storing avatars relies on absolute paths. Modifying it to a new domain could potentially introduce new issues. Saving them as relative paths might be a better option. 🤔

[Octobug]

Thanks for the feedback. It seems that the storing avatars relies on absolute paths. Modifying it to a new domain could potentially introduce new issues. Saving them as relative paths might be a better option. 🤔

@LinkinStars
Yeah, uploaded files should be stored as relative paths. And, for those already deployed in production, the fixing should implement a mechanism to automatically update existing data.

• This could be done by an independent command line tool. or
• During the installation. or
• In the backend code, auto rewrite those stored paths when fetching them.

[Octobug]

@LinkinStars I found that all images in a question, answer or avatar have this issue. I've changed the issue title.

[Octobug]

I would like to solve this issue once the solution is settled. 👀

[calmdev]

Fwiw, I ran into this issue with logo and favicon as well under the branding settings when originally testing how to go about setting up answer.

[LinkinStars]

Thanks for the feedback. It seems that the storing avatars relies on absolute paths. Modifying it to a new domain could potentially introduce new issues. Saving them as relative paths might be a better option. 🤔

@LinkinStars Yeah, uploaded files should be stored as relative paths. And, for those already deployed in production, the fixing should implement a mechanism to automatically update existing data.

• This could be done by an independent command line tool. or
• During the installation. or
• In the backend code, auto rewrite those stored paths when fetching them.

@Octobug Your suggestion is great. We have carefully discussed this issue for some time and believe that providing a command-line solution would be better.

For example, we can offer a functionality like this:

./answer fix image from A to B

This command can modify the domain name in existing data.

The reasons for this are as follows:

1. Existing users may not prefer using relative paths to access images.
2. Answer supports storage plugins, allowing users to upload images to other locations, such like S3 or OSS. In such cases, the saved image may have a different absolute domain path, such as https://s3.east-1.xxx.com/. Of course, this is just a possibility. By providing this command-line tool, we can assist users in making the switch when they want to change their storage and update this part of the address.

Additionally, there is another requirement. We believe it would be helpful to provide an environment variable, ABSOLUTE_IMAGE_PATH, which users can set to true. In this way, Answer will default to using absolute paths for storing images. This consideration is made for existing users who may copy the content to other places where the images won't display if relative paths are used.

Once again, thank you very much for suggesting this feature because we believe it could pose a little big challenge. What do you think? If there are any unclear points, we can continue the discussion.

[Octobug]

@LinkinStars Thank you for your detailed response!

⚠️ I haven't read much of the relevant code yet so I might miss some background knowledge of Answer. I'll make a summary to see if I have understood correctly.

I am also in favor of building this functionality as a ./answer command option.

And let's split a URL into 2 major parts for the convenience in describing:

• PROTOCOL://HOSTNAME:PORT
• PATH
  • BASE_PATH
  • RELATIVE_PATH

I think the ABSOLUTE_IMAGE_PATH could be renamed to IMAGE_PATH_PREFIX (if there will be other types of files in the future, maybe it could be UPLOADS_PATH_PREFIX).

There are 2 major types of uploaded files:

1. For any external storage provided by 3rd-party services (S3, OSS, ...), files are stored as PROTOCOL://HOSTNAME:PORT + PATH in DB with no doubt.
2. For files stored locally, application codes can directly use IMAGE_PATH_PREFIX + RELATIVE_PATH, only RELATIVE_PATH is stored in DB. This usage of IMAGE_PATH_PREFIX can be delivered by documentation:
   • 2.1. By default, the value of IMAGE_PATH_PREFIX is /
   • 2.2. For files need to be moved to a new directory, the value of IMAGE_PATH_PREFIX is BASE_PATH (e.g. new/dirname/)
   • 2.3. For files need to be moved to another self-hosted place different from the Answer instance, the value of IMAGE_PATH_PREFIX is PROTOCOL://HOSTNAME:PORT + BASE_PATH (e.g. https://statics.self-hosted.com/uploads/). Update: Uploading would be a problem. Better not to support it.

Pseudocode:

// When fetching existing data to rewrite:
getImageURL(image) {
    if (isLocalFile(image)) { // an image has a property to be distinguished. e.g. !startsWith('http')
        return IMAGE_PATH_PREFIX + image.path;
    } else {
        return image.path;
    }
}

As for the ./answer fix image from a to b command, I think the from a and to b args are not needed. Because what are stored in DB are always RELATIVE_PATHs.

Update: for files stored in external places, these args are needed.

- Or, are the two args used for auto-migrating uploaded files (mv SRC DST)?
- The description above assumes that an admin move uploaded files manually.

Update:

• IMAGE_PATH_PREFIX is used to fix the existing problem: After this problem is fixed, domain changes don't need any extra operations.
• ./answer migrate image from a to b is used to provide a migration feature: When you change external storage type, migration is inevitable.

[Octobug]

@LinkinStars

This task seems heavy for a Go & Answer beginner (exactly me 🤣). If the coding job is real complicated under your estimation, I think it's better be done by your team.

[LinkinStars]

@Octobug

I think the ABSOLUTE_IMAGE_PATH could be renamed to IMAGE_PATH_PREFIX (if there will be other types of files in the future, maybe it could be UPLOADS_PATH_PREFIX).

I agree. IMAGE_PATH_PREFIX would be better.

Update:

IMAGE_PATH_PREFIX is used to fix the existing problem: After this problem is fixed, domain changes don't need any extra operations.
./answer migrate image from a to b is used to provide a migration feature: When you change external storage type, migration is inevitable.

That's a good summary. You got it.

This task seems heavy for a Go & Answer beginner (exactly me 🤣). If the coding job is real complicated under your estimation, I think it's better be done by your team.

Take it easy. You can try your best, and we will help you solve any problems together. 💪

[Octobug]

Take it easy. You can try your best, and we will help you solve any problems together. 💪

@LinkinStars Okay then, I'll give it another try.

[Octobug]

@LinkinStars

I have another thought on external storage type...

If the uploads dataset is huge, the migration could be very time-consuming because it needs to query & filter out all urls of avatars and urls in texts of questions & answers, and then write them back to DB.

What if Answer saves all types of uploads with relative paths with a prefix marker? Like: :NUM:/uploads/post/57pxxvykQFj.jpeg

:NUM: is a storage url prefix version number. And the version number maps to a URL prefix string, which is used to replace the :NUM: marker in a relative path stored in DB.

The default value of NUM is 0 mapping to "/", it deals with local uploads type. When an Answer admin decides to move the uploads directory to another path, the admin just needs to change 0 to map to "/new/path" after files are settled down.

If the admin adds an external storage, since there is a visit_url_prefix config field for every storage plugin, Answer knows if it is a new prefix. If it is, it increases NUM, and set it as active so that any subsequent uploads use the new active NUM.

Let's say there is a new type of site_info called uploads, it looks like:

{
    "url_prefix_versions": {
        "0": {
            // ":0:/uploads/post/57pxxvykQFj.jpeg"
            "active": true,
            "value": "/"
        },
        "1": {
            // ":1:/uploads/post/57pxxvykQFj.jpeg"
            "active": false,
            "value": "https://s3.east-1.xxx.com/"
        },
        "2": {
            // ":2:/uploads/post/57pxxvykQFj.jpeg"
            "active": true,
            "value": "https://abc.oss-cn-hangzhou.aliyuncs.com/"
        },
        ...
    },

    // other properties...
}

With this solution, those relative paths hardly need to be changed.

The basic migrate operation process is:

1. The admin copies their uploads to another place.
2. Then the admin updates the corresponding NUM's mapping.
3. The admin now can delete uploads in the old place.

And still, ./answer migrate uploads from_a to_b could be treated as the last resort to rewrite paths in DB. And if the version number solution is adopted, this migrate functionality is better to support both command line and the admin UI. It can provide migration status like progress, affected number of urls, etc.

However, migration of large dataset may not be common. The consideration here is whether it is worth implementing such a feature to address a rare scenario.

If this idea is rejected, I will try to implement the previously discussed solution.

[LinkinStars]

@Octobug First of all, thank you very much for presenting the new proposal. We have had a detailed discussion regarding your proposal. It is certain that using external storage for migration would make it somewhat easier.

However, we still have some considerations:

1. For regular users, this additional configuration would require extra understanding, thus increasing the learning curve for them.
2. There would be an additional number in the path, which might appear slightly odd to users.
3. As you also mentioned, this migration is a very infrequent operation and might only be performed once. This point convinced us.

Therefore, based on the reasons stated above, we would prefer to keep the process simple, and thus we suggest sticking with the previous solution would be better.

[Octobug]

@LinkinStars Okay, it was just a thought. 👀

I would like to confirm some details:

The env var

Does this var need to be supported both in Config File and Environment Variable?

• In config file: service_config.upload_url_prefix
  • Used when site is running.
• Environment Variable: UPLOAD_URL_PREFIX
  • Used for installation
• Default value: "/"

It actually affects all kinds of uploads, so I think upload is more proper. And it may include PROTOCOL://HOSTNAME:PORT, so UPLOAD_URL_PREFIX is better than UPLOAD_PATH_PREFIX.

The command

./answer fix upload PREFIX_SRC PREFIX_DST