blossom: implement BUD-05 without optimizations #45
Conversation
|
Was this AI-generated? |
|
No, i just figured it would look dumb if i only included the redirect 307) that i need and not the others. How bad is it? |
|
@sudocarlos , since this has not been merged yet. maybe you can add the headers as well? Just w.Header().Set("Content-Length", strconv.Itoa(bd.Size))
w.Header().Set("Accept-Ranges", "bytes")In As for the redirects, I think that going with code 307 is fine by the way. We can add support for other types of redirects as needed. Honestly, in practice, I doubt that we'll need anything, but 307 redirects any time soon. |
94685d7 to
1f5380f
Compare
|
that worked to add the content-length But blossom.band is still returning HTTP 400, new error: |
|
Blossom.band could infer the content type from the extension as per BUD-04:
This may be actually be a bug since Neither Content-Type nor Content-Length are guaranteed to be be there assuming that "should" in Blossom specs has the same "recommended" meaning as in RFC 2119 (@hzrd149, correct me if I'm wrong). However, providing a proper /HEAD implementation sounds like the right thing to do, and it is an easy enough fix on our side. Can you add the Content-Type header in the same place? |
|
yeaaaa we did it! (mostly you) |
|
I wrote zero code 🤣. Congrats! 🎉 I'll wait for this to be merged and fiatjaf to release a new version and update Haven accordingly. Out of curiosity, did this fix the Primal / IOS bug as well? |
|
After adding |
|
No, the iOS bug still exists |
|
Good stuff. Congrats again.
I'm a bit a lost with this one then. Is there anything obviously in the response body and headers of PUT /{SHA256}, GET /{SHA256} or HEAD /{SHA256} between Khatru 's Blossom server vs Primal and Nostr.band (assuming that both of them are working correctly with Primal on iOS?). If not I think that we'll need to fight this fight another day. |
|
I am using Proxyman on iOS to see whats going on with the packets. Just as with primal.net, primal.iOS is using PUT /media, not PUT /upload. primal.iOS is successful in putting the media on my server and blossom.band but the thumbnail disappears from the note draft like it failed. i don't have this problem in primal.net and the media is successfully mirrored to all blossom servers. |
|
primal.iOS works fine with blossom.primal.net or blossom.band as primary. |
Can you post the response body + headers from Primal Blossom /PUT MEDIA {SHA256} vs the same for Khatru? Are they doing any other requests just after the upload? (I.e., OPTION/HEAD/GET /{SHA256} or maybe listing the media for your pubkey). Is so, any differences in body or headers to Khatru? |
Extension is optional, having content-type/length is required in majority of the cases (ex.: Amethyst uses content type to decide how to render the media), and it is just a good practice overall to serve all relevant headers to tell downloader what they are getting before they start downloading. |
|
Check CORS headers, not sure if you are adding them to any of the endpoints. |
I do agree with you. the specs probably should use MUST here. However… and I know I’m sidetracking a bit into something unrelated to the issue (Khatru will be a good citizen and serve both headers with today’s patch), here’s my two cents from a practical standpoint: I would strongly encourage you not to rely solely on or mandate
If you ask me, while it’s a bit more work for Blossom server developers, I would likely change the specs to assume clients will do the least amount of work possible, and servers should be as lenient as they can. Ideally, the specs would enforce that But this is a much broader discussion, with its own trade-offs.
CORS headers are already being served with a fairly lenient policy: Lines 30 to 40 in 3da898c Unfortunately, the issue isn’t reproducible in the web version of Primal, and there’s plenty of weirdness in the HTTP spec (including additional CORS-related stuff like credentials and exposed headers) that might be necessary depending on what Primal is doing. That’s why I’m curious about the specific Primal requests and Khatru responses on iOS, in comparison to blossom.band or Primal’s own Blossom server. I don’t have an iPhone myself, but hopefully Carlos or one of the Primal devs can shed some light on exactly what their upload service is doing on iOS. |
|
Extension is not optional. Most clients use the extension to decide how to render a media file and if they will download it at all, and that's how it should be because it's simpler and more efficient for everybody. |
This is another thing I’d like to change in the specs. I'm sure @hzdr149 had valid reasons for making extensions optional, like encrypted content in Gift Wrapped messages where we might not want to reveal the file type, but the whole “optional extension” approach is... confusing. For example, a user might upload a JPEG image without an extension and later request It would be much better if extensions were mandatory. Though I still wouldn’t rely on them for By the way, @barrydeen: as part of Haven’s breaking changes / major release where we’ll be separating the Blossom DB, I’ll take the opportunity to also rename the blobs in the Blossom folder to include the file extension. Since I’ll have to move every blob descriptors to the new database anyway, we might as well make our lives easier for when Haven supports uploading / redirecting blobs to S3-like services. |
|
@sudocarlos, congrats for the merge. If you want to troubleshoot the iOS stuff open another bug. I'll get the new version of Khatru updated on Haven as soon as I'm back home today. |
|
@fiatjaf Can you tag this one? |
|
@fiatjaf sorry for jumping in late, but I feel its worth responding to your comment.
File extensions SHOULD be included in the returned "blob descriptor" in the However making the file extension mandatory wont prevent clients from requesting the blob with different or random file extensions. and if servers start rejecting requests because "the ext is wrong" it will start to undermine clients ability to quickly fallback to other servers to find missing blobs In the example @aaccioly mentioned
Its best if the server returns the |
I agree with this. But then again, making the file extension mandatory doesn't mean we can't serve the same blob without an extension (or with a different one) and override the correct Nevertheless, for this to work in practice, I still believe we need to update the specs:
This would resolve the current ambiguity without making discoverability harder. |
Thank you, makes sense and sounds good to me, except I think the blob descriptor should be MUST. I think it should be the server's job to inspect the blob and determine the content-type, if the user doesn't provide one. I don't know what to do if the user provides an invalid extension. Maybe it's ok if the server trusts that or if it overrides that, it will depend on the server and that's ok. |
Adds BUD-05 without optimizations by redirecting with HTTP 307 to /upload as suggested here bitvora/haven#75 (comment)