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

ipaclient: schema cache: Handle malformed server info data gracefully #363

Closed
wants to merge 1 commit into from
Closed

ipaclient: schema cache: Handle malformed server info data gracefully #363

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 3, 2017

As a part of CLI schema cache some data about each previously contacted server
are stored in simple JSON file. The file may get corrupted and became
undecodable for various reasons (parallel access, file system error,
tampering). Since the data are not necessary we should just warn an continue.

https://fedorahosted.org/freeipa/ticket/6578

@stlaz stlaz self-assigned this Jan 3, 2017
self._dict = json.load(sc)
except EnvironmentError as e:
if e.errno != errno.ENOENT:
self._dict = json.loads(sc)
Copy link
Contributor

Choose a reason for hiding this comment

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

sc is a file object, so load() was correct, wasn't it?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, stupid me. I was curious how the file looks like so I had there:
data = sc.read()
json.loads(data)
and printed the data when the error occurred. Obviously, I haven't changed it back properly.
Thanks!

@HonzaCholasta
Copy link
Contributor

Can the corruption also happen for schema files?

@ghost
Copy link
Author

ghost commented Jan 3, 2017

@HonzaCholasta Yes, it can happen but it is already handles correctly: https://github.com/freeipa/freeipa/blob/master/ipaclient/remote_plugins/schema.py#L387

@@ -41,8 +41,13 @@ def _read(self):
try:
with open(self._path, 'r') as sc:
self._dict = json.load(sc)
except EnvironmentError as e:
if e.errno != errno.ENOENT:
except (EnvironmentError, ValueError) as e:
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you should except Exception as e: here, same as in https://github.com/freeipa/freeipa/blob/master/ipaclient/remote_plugins/schema.py#L387

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I will update it for the sake of consistency. But there will be no difference in behaviour.

@HonzaCholasta
Copy link
Contributor

@dkupka, ok, see inline comment.

@stlaz stlaz assigned HonzaCholasta and unassigned stlaz Jan 3, 2017
As a part of CLI schema cache some data about each previously contacted server
are stored in simple JSON file. The file may get corrupted and became
undecodable for various reasons (parallel access, file system error,
tampering). Since the data are not necessary we should just warn an continue.

https://fedorahosted.org/freeipa/ticket/6578
@HonzaCholasta HonzaCholasta added ack Pull Request approved, can be merged pushed Pull Request has already been pushed labels Jan 9, 2017
@HonzaCholasta
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
2 participants