Request response pretty printing #52

Closed
wants to merge 8 commits into
from

Projects

None yet

3 participants

@davidapgar

This adds pretty printing of requests and responses for network traffic. The prettyStringPrinters are asked in reverse order of registration to determine if they know how to pretty print a given request/response type.

Also, moved redacting of fields into the printers, instead of having it hard-coded in the network traffic monitoring

Dave Apgar added some commits Mar 11, 2013
Dave Apgar Set human readable text of http status codes 66c5ac6
Dave Apgar Add PDPrettyStringPrinting Protocol and text implementation
Handles the non-binary MIME types and defines the interface for
other pretty string processors
1b43f75
Dave Apgar Add PrettyStringPrinter registration af76274
Dave Apgar Parse text/binary responses with the default pretty string printer
Instead of hard-coding what is text or binary, if there isn't a pretty
string printer for the content type, treat it as binary
e4bcc8e
Dave Apgar Add JSON pretty printer, and have it handle redacting
The PDDebugger loads this printer by default on monitoring all network
traffic
c2c9a0d
Dave Apgar Move default text pretty printer registration to not depend on a netw…
…ork domain controller

Otherwise, there is a dependency the default instance being used prior to any prettyStringPrinters
being registered (which isn't a requirement)
8e74318
@wlue wlue commented on the diff Mar 12, 2013
ObjC/PonyDebugger/PDNetworkDomainController.m
+ }
+}
+
++ (void)unregisterPrettyStringPrinter:(id<PDPrettyStringPrinting>)prettyStringPrinter;
+{
+ if (!prettyStringPrinters) {
+ return;
+ }
+ @synchronized(prettyStringPrinters) {
+ NSMutableArray *newPrinters = [prettyStringPrinters mutableCopy];
+ [newPrinters removeObjectIdenticalTo:prettyStringPrinter];
+ prettyStringPrinters = newPrinters;
+ }
+}
+
++ (id<PDPrettyStringPrinting>)prettyStringPrinterForRequest:(NSURLRequest *)request
@wlue
wlue Mar 12, 2013

Missing some semicolons in implementation. As per this repo's style, should be:

+ (id<PDPrettyStringPrinting>)prettyStringPrinterForRequest:(NSURLRequest *)request;
@wlue wlue commented on the diff Mar 12, 2013
ObjC/PonyDebugger/PDNetworkDomainController.m
{
- NSString *encodedBody = isBinary ?
- response.PD_stringByBase64Encoding :
- [[NSString alloc] initWithData:response encoding:NSUTF8StringEncoding];
+ id<PDPrettyStringPrinting> prettyStringPrinter = [PDNetworkDomainController prettyStringPrinterForResponse:response withRequest:request];
+
+ NSString *encodedBody;
+ BOOL isBinary;
+ if (!prettyStringPrinter) {
+ encodedBody = responseBody.PD_stringByBase64Encoding;
+ isBinary = YES;
+ }
+ else {
@wlue
wlue Mar 12, 2013

Should put else on same line as brace.

} else {
@wlue wlue commented on the diff Mar 12, 2013
ObjC/PonyDebugger/PDNetworkDomainController.m
}
-
- // If the data isn't UTF-8 it will just be nil;
- self.postData = [[NSString alloc] initWithData:body encoding:NSUTF8StringEncoding];
-
+ else {
@wlue
wlue Mar 12, 2013

Same as above.

@wlue wlue commented on the diff Mar 12, 2013
ObjC/PonyDebugger/PDPrettyStringPrinter.h
+//
+// Created by Dave Apgar on 2/28/13.
+//
+//
+
+
+#import <Foundation/Foundation.h>
+
+@protocol PDPrettyStringPrinting <NSObject>
+
+- (BOOL)canPrettyStringPrintRequest:(NSURLRequest *)request;
+- (BOOL)canPrettyStringPrintResponse:(NSURLResponse *)response withRequest:(NSURLRequest *)request;
+
+- (NSString *)prettyStringForData:(NSData *)data forRequest:(NSURLRequest *)request;
+- (NSString *)prettyStringForData:(NSData *)data forResponse:(NSURLResponse *)response request:(NSURLRequest *)request;
+@end
@wlue
wlue Mar 12, 2013

Newline above here!

@wlue wlue commented on the diff Mar 12, 2013
ObjC/PonyDebugger/PDPrettyStringPrinter.m
+}
+
+- (NSString *)prettyStringForData:(NSData *)data forRequest:(NSURLRequest *)request
+{
+ return [self prettyStringForData:data];
+}
+
+- (NSString *)prettyStringForData:(NSData *)data forResponse:(NSURLResponse *)response request:(NSURLRequest *)request
+{
+ return [self prettyStringForData:data];
+}
+
+@end
+
+@implementation PDJSONPrettyStringPrinter {
+ NSMutableSet *_redactedFields;
@wlue
wlue Mar 12, 2013

Might be a good idea to make this a public property, or make an initWithRedactedFields.

@wlue wlue commented on the diff Mar 12, 2013
ObjC/PonyDebugger/PDPrettyStringPrinter.m
+ NSMutableSet *_redactedFields;
+}
+
+- (id)init
+{
+ self = [super init];
+ if (self) {
+ _redactedFields = [[NSMutableSet alloc] initWithObjects:@"password", nil];
+ }
+ return self;
+}
+
+- (id)initWithRedactedFields:(NSArray *)redactedFields;
+{
+ self = [self init];
+ for (NSString* field in redactedFields) {
@wlue
wlue Mar 12, 2013

Pointer beside name, not type.

for (NSString *field in redactedFields) {
@wlue wlue commented on the diff Mar 12, 2013
ObjC/PonyDebugger/PDPrettyStringPrinter.m
+
+- (BOOL)canPrettyStringPrintRequest:(NSURLRequest *)request
+{
+ NSString *mimeType = [request valueForHTTPHeaderField:@"Content-Type"];
+ return [self canPrettyStringPrintContentType:mimeType];
+}
+
+- (BOOL)canPrettyStringPrintResponse:(NSURLResponse *)response withRequest:(NSURLRequest *)request
+{
+ return [self canPrettyStringPrintContentType:response.MIMEType];
+}
+
+- (NSString *)prettyStringForData:(NSData *)data;
+{
+ NSData *prettyData = data;
+ if (!data) {
@wlue
wlue Mar 12, 2013

Could move this if statement before line 94.

@wlue

Some style nits, just to keep consistent in the code style. I'll find some time to test out the branch, but looks good to me so far. This looks sweet, by the way.

@matthiasplappert matthiasplappert and 1 other commented on an outdated diff Mar 12, 2013
ObjC/PonyDebugger/PDNetworkDomainController.m
@@ -74,6 +75,58 @@ + (Class)domainClass;
return [PDNetworkDomain class];
}
+#pragma mark Pretty String Printing registration and usage
+
+static NSArray *prettyStringPrinters = nil;
+
++ (void)registerPrettyStringPrinter:(id<PDPrettyStringPrinting>)prettyStringPrinter;
+{
+ static dispatch_once_t onceToken;
+ dispatch_once(&onceToken, ^{
+ // Always register the default to differentiate text vs binary data
+ id<PDPrettyStringPrinting> textPrettyStringPrinter = [[PDTextPrettyStringPrinter alloc] init];
+ prettyStringPrinters = [[NSArray alloc] initWithObjects:textPrettyStringPrinter, prettyStringPrinter, nil];
+ });
+
+ @synchronized(prettyStringPrinters) {
+ NSMutableArray *newPrinters = [prettyStringPrinters mutableCopy];
@matthiasplappert
matthiasplappert Mar 12, 2013

This adds the same printer twice if it is the first call: once in the dispatch_once block and the second time in here. It would also make sense to me to make prettyStringPrinters mutable to avoid copying it all the time.

@davidapgar
davidapgar Mar 28, 2013

I fixed the double addition. The rational for making it non-mutable (and copying every time) is to allow the prettyStringPrinters to be used without having to lock (I assume that we can't be sure that it won't be updated at the same time as they are being used to process a request)

Dave Apgar added some commits Mar 12, 2013
Dave Apgar Better handling of intial prettyPrinter registration
Also, add license information to added headers
788cdb8
Dave Apgar Ensure that default prettyPrinters are registered 6bd93f6
@davidapgar

Updated to fix style and clean up a little bit.
I'm closing this, as I made a new pull request with it - it's only two commits different. Please see pull #58

@davidapgar davidapgar closed this Mar 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment