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

sparkey_hash_write returns successfully when given an invalid hash size #19

Closed
stephenmathieson opened this issue Mar 28, 2014 · 4 comments

Comments

@stephenmathieson
Copy link
Contributor

I know this is a bit edge-casey, but when clobbering an existing hash and providing an invalid hash size, sparkey_hash_write incorrectly returns SPARKEY_SUCCESS.

Test case:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sparkey/sparkey.h>
#include <assert.h>

#define check(rc) ({  \
  if (SPARKEY_SUCCESS != rc) { \
    fprintf(stderr, "error: %s (L#%d)\n", sparkey_errstring(rc), __LINE__); \
    exit(1); \
  } \
});

int
main(void) {
  sparkey_logwriter *writer = NULL;
  check(sparkey_logwriter_create(&writer, "test.spl", SPARKEY_COMPRESSION_NONE, 0));
  check(sparkey_logwriter_put(writer, 5, (uint8_t *) "key1", 7, (uint8_t *) "value1"));
  check(sparkey_logwriter_put(writer, 5, (uint8_t *) "key2", 7, (uint8_t *) "value2"));
  check(sparkey_logwriter_put(writer, 5, (uint8_t *) "key3", 7, (uint8_t *) "value3"));

  // write a hash with size 4
  check(sparkey_hash_write("test.spi", "test.spl", 4));
  // write a hash with invalid size
  check(sparkey_hash_write("test.spi", "test.spl", 3456789));

  assert(0 && "wrote test.spi with an invalid size :(");

  return 0;
}
@spkrka
Copy link
Member

spkrka commented Mar 28, 2014

Interesting. It's not really an important bug, since sparkey_hash_write exits early if it notices there's no work to be done, even before it needs to look at the hash size.

I will move the hash size verification earlier in the step (unless it turns out the code gets a lot messier as a result), but I won't make any new release/tag for it since it's somewhat of a contrived example that requires intentional misuse of the api and doesn't actually cause any harm :)

@stephenmathieson
Copy link
Contributor Author

Haha, yes a very contrived example. It came up writing test cases which should result in failure for a binding I'm working on.

No worries regarding a new tag; I can wait for more useful changes :)

@spkrka
Copy link
Member

spkrka commented Mar 29, 2014

Does this fix the issue for you? 52faa2d

@stephenmathieson
Copy link
Contributor Author

#20

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

No branches or pull requests

2 participants