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

Make sure that files that are siblings of directories, are reported as files #982

Conversation

nickvergessen
Copy link
Contributor

Without the patch, the two files are reported to be a collection:

...
	<d:response>
		<d:href>/col/col/test.txt</d:href>
		<d:propstat>
			<d:prop>
				<d:resourcetype>
					<d:collection/>
				</d:resourcetype>
			</d:prop>
			<d:status>HTTP/1.1 200 OK</d:status>
		</d:propstat>
	</d:response>
...
	<d:response>
		<d:href>/dir/child.txt</d:href>
		<d:propstat>
			<d:prop>
				<d:resourcetype>
					<d:collection/>
				</d:resourcetype>
			</d:prop>
			<d:status>HTTP/1.1 200 OK</d:status>
		</d:propstat>
	</d:response>
...

Reported in nextcloud/server#5543 Proposed patch is from @blancjerome I just wrote the test case and cleaned it up a bit...

@nickvergessen nickvergessen changed the title Make sure that files that are children of directories, are reported as files Make sure that files that are siblings of directories, are reported as files Jul 13, 2017
@PVince81
Copy link
Contributor

PVince81 commented Oct 9, 2017

Travis failures, not sure if related:

1) Sabre\HTTP\SapiTest::testSendLimitedByContentLengthStream

TypeError: stream_copy_to_stream() expects parameter 3 to be integer, string given

/home/travis/build/fruux/sabre-dav/vendor/sabre/http/lib/Sapi.php:93

/home/travis/build/fruux/sabre-dav/vendor/sabre/http/tests/HTTP/SapiTest.php:158

@DeepDiver1975
Copy link
Member

Who has to power to merge? @evert (sorry to wake you up) @staabm @DominikTo ?

@staabm
Copy link
Member

staabm commented Oct 9, 2017

I cannnot merge it as long as the build isnt green.

Just restarted the build.

I cannot say whether this fix is correct. Will take your word for it.

@DominikTo
Copy link
Member

Changelog needs update, too.

@DeepDiver1975
Copy link
Member

Changelog needs update, too.

@nickvergessen can you please take care? THX

@nickvergessen
Copy link
Contributor Author

I just saw that you do forward merges? So should I send my PR against 3.2 and add it to the changelog in the 3.2 block?

@DeepDiver1975
Copy link
Member

I just saw that you do forward merges? So should I send my PR against 3.2 and add it to the changelog in the 3.2 block?

@DominikTo @staabm you are deeper involved here - what is the correct workflow? THX
(Or is that documented and I did not read properly? 😉 )

@staabm
Copy link
Member

staabm commented Oct 10, 2017

in case this is considered a bugfix you should merge/cherry-pick it into the lowest/oldest branch which is applicable for it (e.g. oldest yet maintained one) and merge into newer ones.

IIRC if your git fu is great enough you can do this via commandline, no matter what/where the Pull Request is pointing to. its easier for the maintainer when the base branch of this PR is already pointing to the right branch.

…s files

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the fix-invalid-propfind-data-on-infinit-requests branch from 020fa28 to 0cef604 Compare October 10, 2017 11:13
@nickvergessen nickvergessen changed the base branch from master to 3.2 October 10, 2017 11:14
@nickvergessen
Copy link
Contributor Author

nickvergessen commented Oct 10, 2017

Rebased onto 3.2 (version we are using) since I didn't find info what is still supported

3.1 is EOL since June 2017: http://sabre.io/dav/install/

@nickvergessen
Copy link
Contributor Author

3.1 is EOL since June 2017: http://sabre.io/dav/install/

So it seems to be correct

@DeepDiver1975 DeepDiver1975 merged commit 0a21668 into sabre-io:3.2 Oct 10, 2017
@DeepDiver1975
Copy link
Member

and now cherry-pick to master and 3.3? with/without pull request?

@blancjerome
Copy link

blancjerome commented Oct 10, 2017 via email

@nickvergessen
Copy link
Contributor Author

They seem to just merge it forward, so

  • git checkout 3.3
  • git merge --no-ff 3.2
  • git checkout master
  • git merge --no-ff 3.3
  • git push origin 3.3 master

@DeepDiver1975
Copy link
Member

Done

@nickvergessen nickvergessen deleted the fix-invalid-propfind-data-on-infinit-requests branch October 10, 2017 11:37
@nickvergessen
Copy link
Contributor Author

I think you merged 3.2 in master instead of 3.3 ;)

anyway thanks for getting this forward :)

@evert
Copy link
Member

evert commented Oct 10, 2017

Just fyi, the 3.3 branch was intended to just displace the 3.2. It's been backward compatible, but it has a few small improvements. So if you were to drop 3.2 and do a release of the 3.3 branch, things should be fine.

@LorbusChris
Copy link

Can we get a new release with this PR?

@staabm
Copy link
Member

staabm commented Feb 8, 2018

for a new release we might also update the bundled sabre/xml

https://github.com/sabre-io/xml/releases/tag/2.1.0

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.

None yet

9 participants