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

#652: Harden ACL 'save'/'reload' against missing element in server response. #682

Merged
merged 1 commit into from
Feb 26, 2015
Merged

#652: Harden ACL 'save'/'reload' against missing element in server response. #682

merged 1 commit into from
Feb 26, 2015

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Feb 26, 2015

Apparently, the back-end can (in some cases, at least) omit empty 'acl'
elements from the returned resource.

Fixes #652.

Apparently, the back-end can (in some cases, at least) omit empty 'acl'
elements from the returned resource.

Fixes #652.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 26, 2015
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 770f328 on tseaver:652-handle_empty_acls into f0a1157 on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor

dhermes commented Feb 26, 2015

LGTM.

I was kind of unclear in the discussion in #652 about whether this should be happening / other places we can deal with it.

@tseaver
Copy link
Contributor Author

tseaver commented Feb 26, 2015

Having reload() DTRT fixes get_entities(), as well as anything else which calls _ensure_loaded().

FWIW, I don't understand why the back-end would "optimize away" the empty acl key: that seems silly in the extreme (like those 8 bytes are really gonna matter; adding the test to excluded it probably washes out whatever savings there are).

tseaver added a commit that referenced this pull request Feb 26, 2015
#652: Harden ACL 'save'/'reload' against missing element in server response.
@tseaver tseaver merged commit 51e286f into googleapis:master Feb 26, 2015
@tseaver tseaver deleted the 652-handle_empty_acls branch February 26, 2015 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

storage: some acl methods fail on empty (private) acl
4 participants