Regression: cannot post images anymore #117
Comments
|
thanks! this is fixed in e6ad896 i found a few regressions in 2.4.0. i am going to release 2.4.1 very shortly. |
|
and released |
|
For some reason I can still not post images on 2.4.1 (via docker). I will investigate further whether this is an issue on my end some time this week. Perhaps my /uploads folder has wrong permissions, if i interprete your fixes correctly |
|
can you check to see if 1) the file is actually uploaded, 2) what the permissions are? |
|
The image was not uploaded, however ktistec did create a folder-structure for it inside |
|
So, potentially (without knowing crystal at all...), this could have been caused in:
Lastly, of course this could be a docker-related issue. I could try to rebuild the container and creating the |
is the |
Yes, this is exactly what is happening. Th randomly named folder hierarchy is created (owner is root with 755-perms), but the image file is not there |
|
thanks! yeah, i think this is related to the change i made to move the temporary file into place vs. copying the temporary file. i'm wondering if, in some environments, the file is being unlinked after the move despite it having been moved. |
Maybe this can be gathered from the logs? Is there anything i could set to debug from the /system view? If not, i could try to set up a filesystem notify on my server and upload an image |
|
the only thing i can think of are any obvious stack traces or other indicators in the logs. when you do an upload you should see something like the following if it is successful. if it's anything other than 201 then something went wrong: if you see a 201 and there is no stack trace, then i suspect my theory about the file being unlinked is correct, but it would be great to have confirmation. |
|
after looking through the library code for tempfiles and handling uploads, i'm less convinced my theory is correct. if you want to try to revert the change locally, it's four lines of diff: ae321b7#diff-a248dd6a77ab4447934c5af5d08c30ecc5d62d7d876050dedc90236f3793ac14L20 |
|
With current Note: the image Note 2: the path referenced in the error output |
|
Now, running with following patch (from the diff you posted above): diff --git a/src/controllers/uploads.cr b/src/controllers/uploads.cr
index 0e524496..0b0e9e4c 100644
--- a/src/controllers/uploads.cr
+++ b/src/controllers/uploads.cr
@@ -8,7 +8,7 @@ class UploadsController
post "/uploads" do |env|
filename = nil
filepath = nil
- env.params.files.each do |name, part|
+ HTTP::FormData.parse(env.request) do |part|
if part.filename.presence
filename = env.account.actor.id.to_s
filepath = File.join("uploads", *Tuple(String, String, String).from(UUID.random.to_s.split("-")[0..2]))
@@ -17,8 +17,9 @@ class UploadsController
filename = "#{filename}#{extension}"
end
Dir.mkdir_p(File.join(Kemal.config.public_folder, filepath))
- part.tempfile.chmod(0o644) # fix permissions
- part.tempfile.rename(File.join(Kemal.config.public_folder, filepath, filename))
+ File.open(File.join(Kemal.config.public_folder, filepath, filename), "w") do |file|
+ IO.copy(part.body, file)
+ end
end
end
if filename && filepath
diff --git a/src/framework/method.cr b/src/framework/method.cr
index 39216df9..8531c548 100644
--- a/src/framework/method.cr
+++ b/src/framework/method.cr
@@ -18,7 +18,7 @@ module Ktistec
# "application/x-www-form-urlencoded" on activities. see:
# https://github.com/pixelfed/pixelfed/issues/3049
return call_next env unless env.request.method == "POST"
- return call_next env if env.request.path.ends_with?("/inbox")
+ return call_next env if env.request.path == "/uploads" || env.request.path.ends_with?("/inbox")
return call_next env unless env.params.body["_method"]? == "delete"
# switch method and fix URL paramsPosting an image does work indeed and produces the following output in the logs: |
|
perfect, thanks! from that exception and stack trace i can tell that the temporary directory and image directory are on different mounts, and it's not possible to rename across them—the file has to be copied. i'm going to look at one alternative to this, but i may just revert that rename change and go back to copying which is known to work. i appreciate your help with this! |
So this is in fact a docker-related issue then. The uploads directory is a mounted directory from the docker host, while the /tmp directory is container internal. Thank you a lot for the effort! I am going to test the patch some time today and report back here. |
|
not docker-related—it could have happened on any linux host with a separate mount of temporary files. probably pretty common except in my hosting configuration. |
|
no problem! thanks for your help debugging! |
Looks like
2.4.0introduced a little regression, as images attached to posts are not shown anymore after submission.I tried different images with various formats either with and without caption.
The text was updated successfully, but these errors were encountered: