Skip to content
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

Add ASCII85 decoding #244

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Add ASCII85 decoding #244

wants to merge 14 commits into from

Conversation

gregorni
Copy link
Contributor

Related to #61

Feedback is welcome!

src/decoders/ascii85_decoder.rs Outdated Show resolved Hide resolved
CheckerTypes::CheckAthena(athena_checker)
}

#[test]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add more tests for base85 vs ascii85. There should be tests to make sure this only decodes ascii85 and not base85. Also add tests with longer strings and special characters in the plaintext. See the base64 URL tests for an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you give me a base85 string that isn't ascii85? All the online decoders seems to use the same encoding mechanism for both.

src/decoders/ascii85_decoder.rs Outdated Show resolved Hide resolved
src/decoders/ascii85_decoder.rs Outdated Show resolved Hide resolved
src/decoders/ascii85_decoder.rs Outdated Show resolved Hide resolved
@bee-san
Copy link
Owner

bee-san commented Aug 12, 2023

cargo fmt fails :(

@bee-san
Copy link
Owner

bee-san commented Aug 12, 2023

Ahhhh looks like it's just clippy left!! :)

error: this is an outer doc comment and does not apply to the parent module or crate
Error:  --> src/decoders/ascii85_decoder.rs:1:1
  |
1 | / ///! Decode a ascii85 string
2 | | ///! Performs error handling and returns a string
3 | | ///! Call ascii85_decoder.crack to use. It returns option<String> and check with
4 | | ///! `result.is_some()` to see if it returned okay.
  | |___________________________________________________^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_doc_comments
  = note: `-D clippy::suspicious-doc-comments` implied by `-D warnings`
help: use an inner doc comment to document the parent module or crate
  |
1 + //! Decode a ascii85 string
2 + //! Performs error handling and returns a string
3 + //! Call ascii85_decoder.crack to use. It returns option<String> and check with
4 + //! `result.is_some()` to see if it returned okay.
  |

error: could not compile `project_ares` (lib) due to previous error

@gregorni
Copy link
Contributor Author

gregorni commented Aug 13, 2023

So now all the test pass, except the test suite which gets cancelled for taking too long.

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