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

handle-hex-better #612

Merged
merged 1 commit into from
Nov 22, 2024
Merged

handle-hex-better #612

merged 1 commit into from
Nov 22, 2024

Conversation

rpiazza
Copy link
Contributor

@rpiazza rpiazza commented Nov 21, 2024

if hex is being stored in the db as a string - don't convert it to bytes

@chisholm
Copy link
Contributor

generate_value() has a very general name, but seems to do a very specific thing. In words, if backend says "use string type", value is returned; if backend says "use hex type" and that type is not the same as the string type, assume value is hex, decode that to bytes, and return it. The method name should be a lot more specific, I think.

It would seem to let backend authors choose a non-string SQL type for hex properties if they wish and have the datastore handle it gracefully. But currently if they choose anything other than string, it crashes, and there's no way to fix that. If not a string, if they choose a non-binary type, it doesn't make any sense (you'd be inserting a binary value into a non-binary column).

It seems like an idiosyncratic bit of flexibility which no one can use. Why was this attention afforded to hex properties but not binary? Wouldn't you want to be able to store STIX binary values in binary columns too?

I think we should just hard code it to text for now for hex and binary. Flexibility can be added later if it can be made to work.

@rpiazza
Copy link
Contributor Author

rpiazza commented Nov 22, 2024

Yes - I was thinking of it as a general method for all special cases, for which now there is only hex.
I was thinking it could be overridden for a particular database. It only doesn't work for dictionaries...

@rpiazza rpiazza merged commit 10f4117 into with-db-backends Nov 22, 2024
1 of 16 checks passed
@rpiazza rpiazza deleted the handle-hex branch November 22, 2024 14:54
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.

2 participants