Skip to content

Conversation

@virajparimi
Copy link
Contributor

Description

Added check in File.php of dav app in order to ensure that $request->server['HTTP_X_OC_MTIME'] is an integer. If it is not then throws a BadRequest exception.

Related Issue

#27437

Motivation and Context

How Has This Been Tested?

Have not tested it thoroughly.
Chrome and Ubuntu 14.04

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

}
}
else {
throw new BadRequest('expected mtime to be integer');
Copy link
Contributor

Choose a reason for hiding this comment

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

=> "X-OC-MTime header must be an integer" would be a bit clearer

@PVince81
Copy link
Contributor

Code looks good otherwise, thanks !

@virajparimi
Copy link
Contributor Author

@PVince81 done

@PVince81
Copy link
Contributor

Please test with the desktop client, apparently the value is not recognized as int by is_int, you might need to use another approach to detect this.

``'
// => eval: $request->server['HTTP_X_OC_MTIME']
$evalResult = (string[10]) '1491982450';

// => eval: is_int($request->server['HTTP_X_OC_MTIME'])
$evalResult = (bool) '0';

@virajparimi
Copy link
Contributor Author

Ahh, so the $request->server['HTTP_X_OC_MTIME'] is a string and not an integer. Okay, one possible solution is to use strpos to get the position of . if any, otherwise it would return false and then everything else remains same. What do you think?

@PVince81
Copy link
Contributor

strpos is not acceptable because it wouldn't catch other cases where someone sends an invalid value (for example if someone is crazy enough to send a ISO formatted date instead of an int)

individual-it added a commit that referenced this pull request Apr 12, 2017
@individual-it
Copy link
Member

the is_int approach has also the problem that negative values would be accepted
I've changed the File.php so that put() should be easier testable and added some tests.
@Viraj96 I've made a PR to your fork virajparimi#1
You can think of more tests

@PVince81
Copy link
Contributor

PVince81 commented May 4, 2017

any update ?

@virajparimi
Copy link
Contributor Author

@PVince81 Hi, I have been busy with my End Semester Exams. Will work on it once they get over by this weekend. Sorry for the delay caused.

@DeepDiver1975 DeepDiver1975 added this to the 10.1 milestone May 12, 2017
@mrow4a
Copy link
Contributor

mrow4a commented May 24, 2017

Any update here, or can I review?

@individual-it
Copy link
Member

here is an alternative approach #28066 don't throw any errors but simply make sure the value is legal

@mrow4a
Copy link
Contributor

mrow4a commented Jun 27, 2017

This one can be closed since the other approach has been merged right? @PVince81

@PVince81 PVince81 closed this Jun 27, 2017
@DeepDiver1975 DeepDiver1975 modified the milestones: 10.1, development Oct 10, 2017
@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants