Skip to content

fix(list): panic when creation_date_utc is null #182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 25, 2025
Merged

Conversation

kanna5
Copy link
Contributor

@kanna5 kanna5 commented Mar 24, 2025

I was trying rpaste -l and found that it panics.

After some digging, I found that the code used the serde(default) macro to set a default value for the creation date when it's not available.

As this issue of serde points out, the macro is only useful when the field is undefined (instead of null), so it cannot be used here.

Note: I'm using a musl build of the rustypaste server, and it always returns null for the creation date.

Can't use `serde(default)` here because `serde(default)` only handles
when the field is missing (undefined). It will panic if a null value is
found and the field is not defined as optional.
@kanna5 kanna5 requested review from orhun and tessus as code owners March 24, 2025 06:45
@tessus
Copy link
Collaborator

tessus commented Mar 24, 2025

I can't reproduce the panic. Neither on Linux, nor on macOS.

I have to look into this, because I did some research before I implemented it that way, but this was some time ago.
How can your creation date be null? The data is read from the file timestamp. There is no extra metadata that is stored somewhere. This makes no sense.

I only added the default text, so that it shows a text instead of an empty cell (when using pretty output). Also, it can only be not defined, if you use an older server version that doesn't support (and thus send) the creation time.

I'm heading to bed now. I can have a look at it tomorrow afternoon/evening.

@kanna5
Copy link
Contributor Author

kanna5 commented Mar 24, 2025

Thank you for having a look!

Here's how I reproduce it on my Linux system (with docker):

First, clone the rustypaste repository

git clone 'https://github.com/orhun/rustypaste.git'
cd rustypaste

Make some change to the provided config.toml:

  • Set expose_list to true
  • Uncomment auth_tokens

Create the upload directory and make sure its permissions are correct (according to the provided docker-compose.yml)

mkdir upload
sudo chown 1000:1000 upload

Then just run sudo docker compose up -d to pull and spin up the official docker image (which I believe is a musl build). When it's up, it sould be listening on 127.0.0.1:8000

Upload a file:

curl -sS -H 'Authorization: super_secret_token1' -F 'file=@Dockerfile' 127.0.0.1:8000

Check the list returned by the server:

curl -sS -H 'Authorization: super_secret_token1' 127.0.0.1:8000/list

I can see this result:

[{"file_name":"hydrocarbonaceous-charisse.txt","file_size":547,"creation_date_utc":null,"expires_at_utc":null}]

In which the creation_date_utc is null.

Now try with the rpaste cli tool:

git clone 'https://github.com/orhun/rustypaste-cli'
cd rustypaste-cli
cargo build --release

./target/release/rpaste --pretty --list --auth 'super_secret_token1' --server http://127.0.0.1:8000/

I got this error:

IO error: `Failed to read JSON: invalid type: null, expected a string at line 1 column 87`

Here are some clues I currently have.

  • The server code defines creation_date_utc as Option<String>, so it can be null when serialized.

  • The function used to obtain creation date (metadata.created()) has documentation saying it

    will return an Err on platforms or filesystems where it is not available

    which then get converted to None with the Result::ok function call, resulting in null being returned via the http interface.

  • The mentioned issue of serde(default) macro.

  • It looks like the version of musl library used by rust does not provide file creation time, as mentioned in this issue.

Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks legit to me!

TIL: creation time is not supported in Rust musl 👀

ref: rust-lang/rust#125692

Copy link
Collaborator

@tessus tessus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I have also looked into it and your analysis makes sense. The main issue is that musl does not support the statx call and then the other factors lead to the panic.
It seems that this will be solved when rust is upgrading to musl 1.2.5. Until then, this seems like a great solution.
Thanks for the fix.

@tessus tessus merged commit 9620afb into orhun:master Mar 25, 2025
70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants