Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Should `AFHTTPClient` apply it's default values likes `defaultSSLPinningMode` to each enqueued operation? #883

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

OliverLetterer commented Mar 28, 2013

This is a separations and continuation of pull request #878.

There are places in my code where I don't use the provided method -[AFHTTPClient HTTPRequestOperationWithRequest:success:failure:] but am still using -[AFHTTPClient enqueueHTTPRequestOperation:]. It bothers me and it feels wrong to me, that the AFHTTPClient instance is not applying its default values like defaultSSLPinningMode or defaultCredential to an operation in -[AFHTTPClient enqueueHTTPRequestOperation:], if it has not been set explicitly otherwise by the user himself. In case I don't want default values to be applied, I could still add the operation to the operation queue by myself.

This pull request illustrates how this can be achieved for defaultSSLPinningMode with method swizzling on AFURLConnectionOperation. As a note, method swizzling is only used here to keep the public API as light weight as it is right now. The technique is similar to what UIAppearence is using and described by @steipete here.

AFHTTPClient now applies its default pinning mode to all enqueued ope…
…rations if their SSLPinningMode has not been set explicitly.
Contributor

mattt commented Apr 6, 2013

This is an interesting approach, but I don't think it's entirely necessary, nor exactly consistent with expectations.

AFHTTPClient is responsible primarily for three things:

  1. Creating NSURLRequest objects,
  2. Creating AFHTTPRequestOperation objects, and
  3. Scheduling (enqueueing / canceling) AFHTTPRequestOperation objects

At least for me, it would seem unexpected for a property of a generated operation to change an aspect about it after it's been scheduled (aside from the NSOperation state, of course). And that's the way it sort of has to work, as if you were to make this change, it might be expected that default headers and credentials would also be applied dynamically on enqueue. I don't want to go down that path.

Beyond that, I don't anticipate that SSL pinning would be something often changed after initialization, which makes this somewhat unnecessary.

To your point about there being enqueueHTTPRequestOperation, as well as its literal equivalent, self.operationQueue addOperation:, the former is there solely to balance out the two cancellation methods that exist. As it were, a lot of people are surprised to learn that AFHTTPClient indeed has a readonly NSOperationqueue property.

@mattt mattt closed this Apr 6, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment