Support empty password in basic auth

[LukasReschke]

This is a backport of https://github.com/fruux/sabre-dav/issues/596 to the version used by ownCloud. Without this S2S is not working in some server environments.

This patch has been confirmed to work by @jnfrmarks.

Fixes owncloud/core#13398 and owncloud/core#13486

@DeepDiver1975 @karlitschek Without this S2S is broken in some envs. For me this is clearly a 8.0 target.

[jnfrmarks]

I validated this changing the file locally on both owncloud systems I am using for testing.

[LukasReschke]

Upstream commit: fruux/sabre-dav@4910462

[LukasReschke]

@AdamWill FYI – packaging fun.

[LukasReschke]

Please notice that due to the ungraceful handling of failures by S2S this means that if the remote host does not have this patch applied all files are inaccessible for the user…

We really need to make the S2S functionality handling failures more gracefully. @jnfrmarks agreed to test the behaviour when S2S remote servers are going down (etc…) so I'm pretty confident we can expect some tickets about this soon.

[AdamWill]

@LukasReschke thanks for the heads-up!

[jnfrmarks]

No pressure!

[DeepDiver1975]

fine with me 👍

[DeepDiver1975]

@LukasReschke do we have any information from @evert on a release date of 1.8.12 ?

[LukasReschke]

Changelog says "not known": https://github.com/fruux/sabre-dav/blob/29d85943e0098eae11607001e67ec9b4cc2633c1/ChangeLog ;-)

I'm pretty confident though that @evert may know more about that topic ;-)

[icewind1991]

Code looks good 👍

[evert]

When would you guys like it by? I can accommodate.

[DeepDiver1975]

When would you guys like it by? I can accommodate.

We will bundle OC8 RC by the end of this week latest - would be great to get it in by then? 🙊

[evert]

Can do... will try to get this done tomorrow.

[DeepDiver1975]

Can do... will try to get this done tomorrow.

[evert]

Just released it:

https://github.com/fruux/sabre-dav/releases/tag/1.8.12

Not doing a formal announcement, because it's a rather small change that affects no one I think :)

[DeepDiver1975]

THX a lot @evert

[LukasReschke]

No one except us crazy folks ;-)

Thank you @evert. I'll update this PR to use the composer version.

[DeepDiver1975]

@LukasReschke let's pull 1.8.12 via composer instead of this manual changes

[evert]

You guys use composer now?

[LukasReschke]

"It depends". For core we still have our own autoloader (which we plan to replace with 8.1 or so) and for dependencies we use composer. But we use it "our way" which is updating the dependency and then uploading the generated files to this repository.

[LukasReschke]

PR is updated.

[LukasReschke]

Changeset is that big because I dumped an optimized autoloader: https://getcomposer.org/doc/03-cli.md#dump-autoload

[AdamWill]

@LukasReschke FWIW that's kind of a pain for downstream distribution. It effectively hardcodes the file location for every single class you pull in via composer. The way composer works, if a path is specified for a class in autoload_classmap.php, that is the only place Composer will look for it; it'll just flat require() that location and everything will explode if it's not there.

What the 'optimized autoloader' thing does is simply generate an entry in autoload_classmap.php for every single class provided by something pulled in via composer. So once you've done that, it breaks distribution fallback to loading from the system include path.

We really hate that mechanism - to unbundle anything that uses classmap autoloading we just have to symlink the system-wide copy into the expected location, which is kinda horrible.

We have the option of patching out the classmap mechanism from the composer autoloader entirely, but then we have to make sure we unbundle everything that uses classmap such that it can be found systemwide. If we can't do that we have to patch the autoloader to check the location found via classmap and fall through the other mechanisms if it doesn't exist, which is yet more downstream-specific patching...:/

[LukasReschke]

Well – that file was already there before and just not used for all classes: https://github.com/owncloud/3rdparty/blob/85260798f95159e293ad3831131d06cdc37be55c/composer/autoload_classmap.php

I guess that's what you have to fight with anyways when doing some custom packaging magic ;-)

[AdamWill]

@LukasReschke yes, but this makes it much, much worse. The classes that were already listed there before are from composer packages that specify 'classmap' as their autoload mechanism in their composer.json - usually because they use some kind of completely non-logical layout and it's the only practical option. The majority of the files listed there come from mrclay/minify, from which the only class you were actually using is JSMin, of owncloud/core#13052 fame - we didn't have to deal with all that other crap because OC doesn't use it.

We do have to deal with those ones, yes, as I wrote in the initial post - we have to symlink the system wide copies into the expected locations. But usually there's only a few files to deal with. If every single class is in there, it would just be ridiculously unwieldy to go around symlinking every single unbundled file into place; if you keep this, I'll have to switch to one of the 'patch the autoloader' strategies.

[LukasReschke]

Pff, then let me be nice to you and use the regular autoloader…

Did I already mention that I'm not a fan of distro packages? ;-) hides

[LukasReschke]

@AdamWill Happier now? ;-)

[AdamWill]

yay! thanks ;)

If you really want to go this way, it's not a blocker, it's just yet one more on the big pile of PHP Packaging Annoyances - but I'd really appreciate if you maybe did a bit of profiling and checked that it actually improves performance appreciably, at least, rather than just throwing it in on general principles :)