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

test: Add strand_with_nul_should_panic #4071

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

winksaville
Copy link

@winksaville winksaville commented May 21, 2024

Todo:

-[x] from core/ run cargo test sql::value::serde::ser::value::tests::strand_with_nul_should_panic -- --exact --nocapture
-[x] from core/ run cargo test
-[X] from core/ run cargo test --release sql::value::serde::ser::value::tests::strand_with_nul_should_panic -- --exact --nocapture

-[] from / run cargo test --release FAILING:

wink@3900x 24-05-23T06:13:45.641Z:~/prgs/SurrealDB/surrealdb (test-strand_with_nul_should_panic)
$ cargo test --release
warning: unused import: `crate::key::debug::sprint_key`
  --> core/src/kvs/tx.rs:31:5
   |
31 | use crate::key::debug::sprint_key;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default
...

warning: function `debug_builds_contain_debug_message` is never used
  --> tests/cli_integration.rs:65:5
   |
65 |     fn debug_builds_contain_debug_message(addr: &str, creds: &str, ns: &Ulid, db: &Ulid) {
   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(dead_code)]` on by default

warning: `surreal` (bin "surreal" test) generated 2 warnings (run `cargo fix --bin "surreal" --tests` to apply 1 suggestion)
warning: `surreal` (test "cli_integration") generated 1 warning
warning: `surreal` (bin "surreal") generated 2 warnings (2 duplicates)
    Finished `release` profile [optimized] target(s) in 0.25s
     Running unittests src/main.rs (target/release/deps/surreal-06baae2f5dec9685)

running 7 tests
test cli::validator::tests::test_func_targets ... ok
test cli::validator::tests::test_net_targets ... ok

...

test cli_integration::start_tls ... ok
test cli_integration::test_server_second_signal_handling ... ok
test cli_integration::node ... ok
test cli_integration::test_temporary_directory ... FAILED

failures:

---- cli_integration::test_temporary_directory stdout ----
Server process dropped! Assuming error happend
Server STDOUT: 

Server STDERR: 
error: invalid value '/tmp/TELL-ME-THIS-FILE-DOES-NOT-EXISTS' for '--temporary-directory <TEMPORARY_DIRECTORY>': Ensure the path exists

For more information, try '--help'.

2024-05-23T06:14:18.149401Z ERROR cli_integration::common::server: server output: error: invalid value '/tmp/TELL-ME-THIS-FILE-DOES-NOT-EXISTS' for '--temporary-directory <TEMPORARY_DIRECTORY>': Ensure the path exists

What is the motivation?

A Strand should not have any nul, '\0', characters.

What does this change do?

This tests that a panic should happen if a string to be serialized contains a null ('\0') byte.

Testing from within core/:

$ cargo test sql::value::serde::ser::value::tests::strand_with_nul_should_panic -- --exact --nocapture
warning: unused import: `crate::fflags::FFLAGS`
 --> core/src/kvs/tests/lq.rs:1:5
  |
1 | use crate::fflags::FFLAGS;
  |     ^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: `surrealdb-core` (lib test) generated 1 warning (run `cargo fix --lib -p surrealdb-core --tests` to apply 1 suggestion)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.20s
     Running unittests src/lib.rs (/home/wink/prgs/SurrealDB/surrealdb/target/debug/deps/surrealdb_core-402b08e0fdbff83e)

running 1 test
thread 'sql::value::serde::ser::value::tests::strand_with_nul_should_panic' panicked at core/src/sql/strand.rs:87:9:
assertion failed: !s.contains('\0')
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test sql::value::serde::ser::value::tests::strand_with_nul_should_panic - should panic ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 1219 filtered out; finished in 0.00s

What is your testing strategy?

Add one or more should_panic tests so nul byte handling is more thoroughly tested.

Is this related to any issues?

Issue "Bug: Strand does not check for nul bytes in release mode" #4076
depending on what is decided this may or may not close the issue.

Does this change need documentation?

  • No documentation needed

Have you read the Contributing Guidelines?

This tests that a panic occurs if a string to be serialized
contains one or more null bytes ('\0'). Since a `debug_assert` in
`fn serialize<S>` of core/src/sql/strand.rs the test must be ignored
for `--release` builds. Hence the addition of `[cfg(debug_assertions)]`
attribute.
@winksaville winksaville force-pushed the test-strand_with_nul_should_panic branch from 9785d9b to 30c51bd Compare May 23, 2024 04:39
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.

None yet

1 participant