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

Receiving URL of form https://@dav.box.com/dav #79

Closed
MrNoodle opened this issue Oct 6, 2014 · 17 comments
Closed

Receiving URL of form https://@dav.box.com/dav #79

MrNoodle opened this issue Oct 6, 2014 · 17 comments

Comments

@MrNoodle
Copy link
Contributor

MrNoodle commented Oct 6, 2014

Possibly related to #75.

In this case, the username is a full email address so it has an @ in it. Oddly, I created an account on the same server (which also is a full email address) as this user and it works fine for me. It seems that sometimes the user is disappearing from the URL though I can't figure out a pattern for it. It happens consistently for this user but not for me.

@mikeabdullah
Copy link
Collaborator

So this is a URL vended out by a directory listing?

@MrNoodle
Copy link
Contributor Author

MrNoodle commented Oct 7, 2014

I guess so, yeah. I had it logging at the point that CK2OpenPanel returns it but all the URLs it gets come from directory listings.

@mikeabdullah
Copy link
Collaborator

All URLs the WebDAV protocol vends out should be going via the +URLByReplacingUserInfoInURL:withUser: method. So three possibilities come to mind:

A) An empty username is being fed into this method
B) This method is being bypassed for certain URLs
C) A particular username confuses this method into screwing up

There's tests against C. Presently I suspect A.

@MrNoodle
Copy link
Contributor Author

MrNoodle commented Oct 7, 2014

Ok, after more investigation, I think this may be something that was fixed in more recent versions of CK2. Checking, the user is using an older version whereas I'm doing my test on the latest. Seems like I can replicate his problem running on that version on my test machine. I'll see if I can get him to run the latest as well to see if the problem goes away.

That said, #75 is happening for me running the latest.

@mikeabdullah
Copy link
Collaborator

In which case, since you can repro #75, can you set a breakpoint on line 100 of CKWebDAVProtocol.m and see if you can catch it in action there?

@MrNoodle
Copy link
Contributor Author

MrNoodle commented Oct 7, 2014

Ok, it seems _user is nil and therefore it ends up stripping the user out. Anything else I should check?

@mikeabdullah
Copy link
Collaborator

Oh dear, this is a knotty mess. _user gets filled in in response to an authentication challenge. But I suppose there's two cases where this might not happen:

  1. DAVKit never issues us a challenge
  2. The client chooses a behaviour other than CK2AuthChallengeUseCredential, e.g. NSURLSessionAuthChallengePerformDefaultHandling instead

Are you able to isolate which of the above is the case?

@MrNoodle
Copy link
Contributor Author

MrNoodle commented Oct 7, 2014

Is there a good spot to check for this?

@MrNoodle
Copy link
Contributor Author

MrNoodle commented Oct 7, 2014

Ok, so breaking in -webDAVSession:didReceiveChallenge:..., the disposition does seem to be CK2AuthChallengeUseCredential. I don't see any cases of it being otherwise.

Also, it seems that it doesn't happen when I step through, possibly implying something timing based?

@mikeabdullah
Copy link
Collaborator

Ugh, a timing-based bug is not what we want.

I wonder if I've been treating this all wrong in the API. Let's rewind:

Imagine getting a directory list for http://example.com/myfolder/
What you get back likely depends on the user that you log in as. So if you're writing an app which needs to do some keying or caching based on URL, and the user might log in to multiple accounts across that, then you benefit from getting back unique URLs which include the username.

I think everyone else gets no benefit from the username, and just wants consistency. So I'm thinking, rather than CK attempting to figure out which username is being used for a particular operation, how about we just respect whatever user is specified in the original request URL?

  • Clients which don't specify a user and have simple needs don't get back a user, and are happy
  • Clients which know the user upfront can specify it in the URL, and will get that back again in the results, and are happy
  • Complex clients to whom the user matters as discussed above, and might not know if upfront, can take on responsibility for manipulating the URLs they get back if needed

How would that arrangement suit you?

@MrNoodle
Copy link
Contributor Author

MrNoodle commented Oct 7, 2014

That seems reasonable. That does mean that I do need to specify it up front, which is fine. I had just assumed it would work fine if the user was provided during authentication. You might need to warn other people that they may need to change their clients to compensate.

@MrNoodle
Copy link
Contributor Author

MrNoodle commented Oct 7, 2014

Also, I did notice when tracing through that the user name was intact, it just got erased. Maybe an alternative is to not re-write the URL if the user is nil?

Actually, I should elaborate: the case I am seeing this is not via the open panel. I do have an URL in this case with the username specified so that is fed in to CK2FileManager. It gets erased because _user isn't set. I'm not sure if I've seen this with CK2OpenPanel where I do not supply the user in the initial URL. I'll test that more to see if I can replicate it then.

@mikeabdullah
Copy link
Collaborator

Alright, let me try that out on a branch since it's potentially breaking

mikeabdullah added a commit that referenced this issue Oct 7, 2014
Instead, the username passed in in the original URL is respected. Any
smarts needed beyond that are the client’s responsibility.
As discussed in issue #79
@mikeabdullah
Copy link
Collaborator

@MrNoodle please can you give the simplify-user-handling branch a go, and let me know how that gets on.

@MrNoodle
Copy link
Contributor Author

MrNoodle commented Oct 7, 2014

Seems to work fine for me. Tested both sending in an URL with a user and without and it followed the format of the sent in URL.

Will this be merged into v2.x-beta?

@mikeabdullah
Copy link
Collaborator

Excellent! I'll give people a little warning and time, and then merge it into the 2.x branch. Is that OK by you, or are you keen to get it merged straight away?

@MrNoodle
Copy link
Contributor Author

MrNoodle commented Oct 7, 2014

Actually, I have my own branch off of v2.x-beta that has a couple of minor changes for myself (like getting rid of warnings since I'm running a later version of Xcode). I can just merge into that branch and use that. Either that or I can cherry-pick the change. Either way, that's fine.

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

No branches or pull requests

2 participants