Support replacement of IP address resolutions via command line flag

net/dns/mapped_ip_resolver.h

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef NET_DNS_MAPPED_HOST_RESOLVER_H_
-#define NET_DNS_MAPPED_HOST_RESOLVER_H_
+#ifndef NET_DNS_MAPPED_IP_RESOLVER_H_
+#define NET_DNS_MAPPED_IP_RESOLVER_H_
 
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
-#include "net/base/host_mapping_rules.h"
+#include "base/memory/weak_ptr.h"
+#include "net/base/ip_mapping_rules.h"
 #include "net/base/net_export.h"
 #include "net/dns/host_resolver.h"
 
 namespace net {
 
 // This class wraps an existing HostResolver instance, but modifies the
-// request before passing it off to |impl|. This is different from
-// MockHostResolver which does the remapping at the HostResolverProc
-// layer, so it is able to preserve the effectiveness of the cache.
-class NET_EXPORT MappedHostResolver : public HostResolver {
+// resolution response by inserting or replacing IP addresses before returning

[wtc, 2014/02/11 23:52:30]

You only implemented the "PREFACE" directive, right? So you can't really replace
IP addresses. Replacing means you also delete the replaced address.

[jar (doing other things), 2014/02/15 21:14:56]

I added clarifying comments.

+// it.
+class NET_EXPORT MappedIPResolver : public HostResolver {

[wtc, 2014/02/11 23:52:30]

Nit: "IPResolver" is a poor name because there is no such thing as IP
resolution. That is, an IP address is the result of host name resolution.

Maybe this should be named "IPMappingHostResolver".

[jar (doing other things), 2014/02/15 21:14:56]

As per offline discussion.... I parsed the names (that lead to this name)
differently.

I parsed MappingHostResolver
as a "Resolver" that performed "Mapping of Hosts"

Hence, I called tihs a MappingIPResolver
as a "Resolver" that performs "Mapping of IP addresses"

Your parsing seems to be that MappingHostResolver is
a "HostResolver" that performs some "Mapping"

To be honest, both alternatives for the original name are a bit clunky, and make
it hard to see a clear pattern.

I think your suggestion might be better... but it doesn't match well with the
existing class, which perhaps should then be:
HostMappingHostResolver
or
NameMappingHostResolver
so we could be:
IPMappingHostResolver

I could probably change the existing class name to move in sync if that is what
you'd like.

Comments?


On 2014/02/11 23:52:30, wtc wrote:

  public:
   // Creates a MappedHostResolver that forwards all of its requests through

[wtc, 2014/02/11 23:52:30]

MappedHostResolver => MappedIPResolver

[jar (doing other things), 2014/02/15 21:14:56]

Done.

   // |impl|.
-  explicit MappedHostResolver(scoped_ptr<HostResolver> impl);
-  virtual ~MappedHostResolver();
+  explicit MappedIPResolver(scoped_ptr<HostResolver> impl);
+  virtual ~MappedIPResolver();
 
-  // Adds a rule to this mapper. The format of the rule can be one of:
-  //
-  //   "MAP" <hostname_pattern> <replacement_host> [":" <replacement_port>]
-  //   "EXCLUDE" <hostname_pattern>
-  //
-  // The <replacement_host> can be either a hostname, or an IP address literal,
-  // or "~NOTFOUND". If it is "~NOTFOUND" then all matched hostnames will fail
-  // to be resolved with ERR_NAME_NOT_RESOLVED.
-  //
+  // Adds a rule to our IP mapper rules_.
+  // See ip_mapping_rules.h for syntax and semantics.
   // Returns true if the rule was successfully parsed and added.
   bool AddRuleFromString(const std::string& rule_string) {
     return rules_.AddRuleFromString(rule_string);
   }
 
   // Takes a comma separated list of rules, and assigns them to this resolver.

[wtc, 2014/02/11 23:52:30]

Typo: comma => semicolon

[jar (doing other things), 2014/02/15 21:14:56]

Done.

   void SetRulesFromString(const std::string& rules_string) {
     rules_.SetRulesFromString(rules_string);
   }
 
   // HostResolver methods:
   virtual int Resolve(const RequestInfo& info,
                       RequestPriority priority,
                       AddressList* addresses,
                       const CompletionCallback& callback,
                       RequestHandle* out_req,
                       const BoundNetLog& net_log) OVERRIDE;
   virtual int ResolveFromCache(const RequestInfo& info,
                                AddressList* addresses,
                                const BoundNetLog& net_log) OVERRIDE;
   virtual void CancelRequest(RequestHandle req) OVERRIDE;
   virtual void SetDnsClientEnabled(bool enabled) OVERRIDE;
   virtual HostCache* GetHostCache() OVERRIDE;
   virtual base::Value* GetDnsConfigAsValue() const OVERRIDE;
 
  private:
-  // Modify the request |info| according to |rules_|. Returns either OK or
-  // the network error code that the hostname's resolution mapped to.
-  int ApplyRules(RequestInfo* info) const;
+  // Modify the list of resolution |addresses| according to |rules_|, and then
+  // calls the |original_cb| with network error code |rv|.
+  void ApplyRules(const CompletionCallback& original_cb,

[wtc, 2014/02/11 23:52:30]

Nit: it may be nice to be verbose and name this parameter "original_callback".

[jar (doing other things), 2014/02/15 21:14:56]

Done.

+                  AddressList* addresses,
+                  int rv) const;
 
   scoped_ptr<HostResolver> impl_;
+  IPMappingRules rules_;
 
-  HostMappingRules rules_;
+  base::WeakPtrFactory<MappedIPResolver> weak_factory_;
+  DISALLOW_COPY_AND_ASSIGN(MappedIPResolver);
 };
 
 }  // namespace net
 
-#endif  // NET_DNS_MAPPED_HOST_RESOLVER_H_
+#endif  // NET_DNS_MAPPED_IP_RESOLVER_H_