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

audit: hash time.Time values in map fields #2689

Merged
merged 1 commit into from
May 8, 2017
Merged

audit: hash time.Time values in map fields #2689

merged 1 commit into from
May 8, 2017

Conversation

mitchellh
Copy link
Contributor

@mitchellh mitchellh commented May 8, 2017

This enables audit.Hash to hash time.Time values that may exist as
direct fields in the map. This will error (instead of panic) for any
time.Time values that don't occur within map values. For example, this
does not support a time.Time within a slice. If that needs to be
supported then modifications will need to be made.

This also requires an update to reflectwalk (included in this PR). This
is a minimal change that allows SkipEntry to signal to skip an entire
struct. We do this because we don't want to walk any of time.Time since
we handle it directly.

Fixes #2687

@@ -232,6 +270,7 @@ func (w *hashWalker) Primitive(v reflect.Value) error {
mk := w.csData.(reflect.Value)
m.SetMapIndex(mk, resultVal)
default:
println(w.loc)
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this line?


// Walk it as if it were a primitive value with the string value.
// This will replace the currenty map value (which is a time.Time).
if err := w.Primitive(reflect.ValueOf(v.Interface().(time.Time).String())); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

This puts out a Go-style string -- can you .Format(time.RFC3339Nano) it?

Copy link
Member

Choose a reason for hiding this comment

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

(I realize that since it gets hashed it basically doesn't really matter since you're unlikely to try to decode the value, but for consistency...)

This enables audit.Hash to hash time.Time values that may exist as
direct fields in the map. This will error (instead of panic) for any
time.Time values that don't occur within map values. For example, this
does not support a time.Time within a slice. If that needs to be
supported then modifications will need to be made.

This also requires an update to reflectwalk (included in this PR). This
is a minimal change that allows SkipEntry to signal to skip an entire
struct. We do this because we don't want to walk any of time.Time since
we handle it directly.
@jefferai
Copy link
Member

jefferai commented May 8, 2017

Thanks!

@jefferai jefferai merged commit 4dc061e into master May 8, 2017
@jefferai jefferai deleted the b-hash-time branch May 8, 2017 18:06
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