supabase / storage

S3 compatible object storage service that stores metadata in Postgres
https://supabase.com/docs/guides/storage
Apache License 2.0
764 stars 107 forks source link

Storage v3 - Resumable uploads error #331

Closed DanielAlcaraz closed 1 year ago

DanielAlcaraz commented 1 year ago

Bug report

Describe the bug

I was reading the blog about the new storage features. I found interesting that you implemented the Tus protocol. I was trying to build a simple demo with Uppy as you show in the blog but I get a 404 error.

https://supabase.com/blog/storage-v3-resumable-uploads#deep-dive-into-resumable-uploads

To Reproduce

I created a simple Codesandbox with Svelte and I added the code inside the layout file:

https://codesandbox.io/p/sandbox/muddy-cache-xr8ct2?welcome=true

I didn't add the anon key and my project ref.

I get this error: {"message":"Route POST:/upload/resumable not found","error":"Not Found","statusCode":404}

I guess the URL is not correct? But I could't figure out how to make this work.

Expected behavior

I expect to be able to upload a JPG inside my bucket folder.

Additional context

I also tried to create an upload URL with the Js Client using store.createSignedUploadUrl. It doesn't give the HTTP error anymore but I get an error message inside the Uppy component saying the Location header is missing.

The bucket is public and I created this 2 policies:

 create policy "Public Access Bucket Images Get"
on storage.objects for select
using ( bucket_id = 'users' );

create policy "Public Access Bucket Images Post"
on storage.objects for insert
with check ( bucket_id = 'users' );

I don't know if they are correct. I just want to allow to upload files for the authenticated users. I tried to make the buket open to test if everything works well.

Thank you.

GaryAustin1 commented 1 year ago

I believe you have to request access for this at the moment (at least I have not seen anything saying it is mass released yet). Because this is a major update, weโ€™re rolling it out gradually over the next month. You will receive a notification in your dashboard when the feature is available for project. Reach out to us if you want early access to this feature.

So just confirming you have done that.

fenos commented 1 year ago

Yes, that's correct! If you reach out to us we will enable it for you

We are going to release it globally very soon

Obeyed commented 1 year ago

@fenos Are there any resources on how to use resumable uploads when self-hosting?

I tried to follow this guide: https://supabase.com/docs/guides/storage/uploads#resumable-upload but hit two challenges:

Mixed Content: The page at 'https://xxxx' was loaded over HTTPS, but requested an insecure XMLHttpRequest endpoint 'http://<a-kong-gateway-domain>/upload/resumable/<some-resumable-upload-id>'. This request has been blocked; the content must be served over HTTPS.

One thing to notice as well is that the storage/v1 part of the url has been stripped for some reason. I assume it would have failed regardless of it being requested with http instead of https.

I'm using the supabase/storage-api:v0.41.5 docker image.

fenos commented 1 year ago

Hi @Obeyed

The storage server is set up to run behind a reverse proxy or load balancer, that forwards the x-forwarded headers. but it can be run on localhost too, there are 2 simple steps to run TUS with a reverse proxy.

1) Make sure the reverse proxy is setting x-forwarded headers

In your case, it seems you are using Kong, but it is not forwarding the headers correctly. If you are using this setup on your local machine without a domain you might need to use this trick that we implemented on our local development: https://github.com/supabase/cli/blob/main/internal/start/templates/kong.yml#L88

In production, the headers should be sent properly with the correct domain and host so you don't need the above change.

2) Customise the TUS_URL_PATH env

in order to have the storage/v1 included you need to set the env variable TUS_URL_PATH on the storage-api server to /storage/v1/upload/resumable

let me know if this works for you ๐Ÿ‘

Obeyed commented 1 year ago

@fenos Really appreciate the response and help! Got it working. :raised_hands:

I added the TUS_URL_PATH and didn't have to include the /storage/v1 in the kong config. So I ended up with the following.

When running locall, I added this to the kong.yml under the storage service:

      - name: request-transformer
        config:
          add:
            headers:
              - "Forwarded: host=localhost:8000;proto=http"

On a server with nginx as reverse proxy, I tried this, and it worked. (I couldn't figure out how to make nginx / kong forward the protocol correctly without this).

      - name: request-transformer
        config:
          add:
            headers:
              - "Forwarded: proto=https"

I presume that if I'm running the servers behind a load balancer, then I would add the "host" part and set it to the domain pointing to the load balancer.

If you know any resources on what the best configuration with nginx is, then please do share. I'm using Docker Swarm to orchestrate the services.

Do you know why there's a hard limit of 6mb for the chunk size? The docs just say this is currently a limitation with the storage-api, but adds no further explanation. Would be cool to understand more. :smile: Thanks again!

fenos commented 1 year ago

@Obeyed Glad you got it working!

In terms of configurations, as far as the x-forwarded-proto and x-forwarded-host are sent over to the storage-api the corresponding host and protocol will be used.

Do you know why there's a hard limit of 6mb for the chunk size? The docs just say this is currently a limitation with the storage-api, but adds no further explanation. Would be cool to understand more. ๐Ÿ˜„ Thanks again!

Sure, the reason behind the 6MB limit is that S3 imposes the part size upload to be a minimum of 5MB and a maximum of 5GB we chose to go with 6MB since for large files seems the right middle ground

https://docs.aws.amazon.com/AmazonS3/latest/userguide/qfacts.html

with a 6MB chunk and 1000 parts limits we can handle up to 60GB file uploads

urania-dev commented 8 months ago

@fenos Really appreciate the response and help! Got it working. ๐Ÿ™Œ

I added the TUS_URL_PATH and didn't have to include the /storage/v1 in the kong config. So I ended up with the following.

When running locall, I added this to the kong.yml under the storage service:

      - name: request-transformer
        config:
          add:
            headers:
              - "Forwarded: host=localhost:8000;proto=http"

On a server with nginx as reverse proxy, I tried this, and it worked. (I couldn't figure out how to make nginx / kong forward the protocol correctly without this).

      - name: request-transformer
        config:
          add:
            headers:
              - "Forwarded: proto=https"

I presume that if I'm running the servers behind a load balancer, then I would add the "host" part and set it to the domain pointing to the load balancer.

If you know any resources on what the best configuration with nginx is, then please do share. I'm using Docker Swarm to orchestrate the services.

Do you know why there's a hard limit of 6mb for the chunk size? The docs just say this is currently a limitation with the storage-api, but adds no further explanation. Would be cool to understand more. ๐Ÿ˜„ Thanks again!

This. i looked everywhere, tinkered with traefik for six hours, then i remembered reading this. it works <3

jackson-jackson commented 8 months ago

@fenos do we still need to request access?

GaryAustin1 commented 8 months ago

@jackson-jackson v3 should be out on instances for awhile now. Certainly any new instances in the past couple of months. You MAY need to upgrade the database or pause/restore an older instance, but not sure on this specific update.

olee commented 8 months ago

I'm having the same issue with uploads to storage not returning the correct proxy url: image

I tried the fix @Obeyed proposed here but it still doesn't work at all. Why would we even have to add the "Forwarded" header when there are already proper "X-Forwarded-*" headers sent to storage api?

I also tried fully inserting the Forwarded header with a dynamic value for host, but for some reason kong does not seem to insert any value here:

      - name: request-transformer
        config:
          add:
            headers:
              - 'Forwarded: host=$(headers["X-Forwarded-Host"]):8000/storage/v1;proto=http'

EDIT

I checked the used @tus/server library and noticed that is seems they themselves are not properly handling all the X-Forwarded headers properly yet, but I think it should be possible to fix that on the storage implementation side here: https://github.com/supabase/storage/blob/830ea71d90c079f487f41d878aafed935f87d25e/src/http/routes/tus/lifecycle.ts#L54-L68

That would also make this here unnecessary: https://github.com/supabase/cli/blob/b380dfbc53d494a7b2e0c0f55b9503c6fd7d9796/internal/start/templates/kong.yml#L98-L102

EDIT 2

I at least found out what the issue was with Forwarded header: Because there is an env substitution happening to kong.yaml, the $(headers.host) was being replaced at that time instead of inside kong. I escaped it like this and now it's working:

      - name: request-transformer
        config:
          add:
            headers:
              - 'Forwarded: host=\$(headers.host)/storage/v1;proto=http'

I still think it would be better to change the code to properly handle the proxy headers though.

AntonOfTheWoods commented 3 months ago

This is more than a little infuriating...

So it was actually working (a file is definitely uploaded and visible in the studio!) without the Forwarded: ... and then once it stops working, namely it goes from correctly using https to suddenly using http for the supabase/kong generated links. It doesn't seem to matter what I put in the kong config, it won't come back. Setting NODE_ENV === 'production' doesn't seem to help either.

I have tried adding this to kong in my kubernetes helm chart (running behind a ingress-nginx controller):

      - name: request-transformer
        config:
          add:
            headers:
              - "Forwarded: proto=https"

But this just has no effect. I checked and the kong config file is definitely updated in the container... I am still getting "x_forwarded_proto":"http" in the logs of the storage container.

I also note that the kong docs mention this:

The X-Forwarded-* fields are non-standard header fields written by Nginx to inform the upstream about client details and canโ€™t be overwritten by this plugin. If you need to overwrite these header fields, see the [Post-function plugin](https://docs.konghq.com/hub/kong-inc/post-function/how-to/).

For the docs of the request-transformer plugin... So I'm completely lost.

EDIT: Unbelievable! Somehow I missed that there was a cached value in localStorage with the wrong URL, and it wasn't getting updated... So it appears that it does work... Sigh.

btouchard commented 2 weeks ago

Hello I have Supabase with Storage behind a Traefik reverse proxy. I spent 10 hours trying to solve this problem after updating Supabase Storage. Impossible to make TUS work with Storage version 1.10.1 or higher, so I used version 1.9.1, which works, if you configure Kong with this:

      - name : request-transformer
        config :
          add :
            headers :
              - 'Forwarded: host=\$(headers.host)/storage/v1;proto=https'

and TUS_URL_PATH=/upload/resumable in environment variables

matthieumasse commented 2 weeks ago

Hello I have Supabase with Storage behind a Traefik reverse proxy. I spent 10 hours trying to solve this problem after updating Supabase Storage. Impossible to make TUS work with Storage version 1.10.1 or higher, so I used version 1.9.1, which works, if you configure Kong with this:

      - name : request-transformer
        config :
          add :
            headers :
              - 'Forwarded: host=\$(headers.host)/storage/v1;proto=https'

and TUS_URL_PATH=/upload/resumable in environment variables

Same for me. 4 hours to finally downgrade to 1.9.1

tothster commented 2 weeks ago

Hello I have Supabase with Storage behind a Traefik reverse proxy. I spent 10 hours trying to solve this problem after updating Supabase Storage. Impossible to make TUS work with Storage version 1.10.1 or higher, so I used version 1.9.1, which works, if you configure Kong with this:

      - name : request-transformer
        config :
          add :
            headers :
              - 'Forwarded: host=\$(headers.host)/storage/v1;proto=https'

and TUS_URL_PATH=/upload/resumable in environment variables

Same for me. 4 hours to finally downgrade to 1.9.1

Didn't work for me even with the modifications in kong.yml, TUS_URL_PATH in .env, and the downgrade. The environment variable is to be included in the .env file on the docker folder, right? My browser keeps hitting localhost:8000/storage/v1/upload/resumable.