CrossOriginAccessControl: separate access checks and error message generation

third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h

Index: third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h
diff --git a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h
index ac2530c9805ca9f3d99bb8adb3df41f0935dddaa..52c4e55d0b49b8b3a27830f245015ced0cddbbe5 100644
--- a/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h
+++ b/third_party/WebKit/Source/core/fetch/CrossOriginAccessControl.h
@@ -48,33 +48,103 @@ class CrossOriginAccessControl {
   STATIC_ONLY(CrossOriginAccessControl);
 
  public:
-  // Given the new request URL, returns true if
+  // Enumerating the error conditions that the CORS
+  // access control check can report, including success.
+  //
+  // See |checkAccess()| and |accessControlErrorString()| which respectively
+  // produce and consume these error values, for precise meaning.
+  enum AccessStatus {
+    kAccessAllowed,
+    kInvalidResponse,
+    kAllowOriginMismatch,
+    kSubOriginMismatch,
+    kWildcardOriginNotAllowed,
+    kMissingAllowOriginHeader,
+    kMultipleAllowOriginValues,
+    kInvalidAllowOriginValue,
+    kDisallowCredentialsNotSetToTrue,
+  };
+
+  // Enumerating the error conditions that CORS preflight
+  // can report, including success.
+  //
+  // See |checkPreflight()| methods and |preflightErrorString()| which
+  // respectively produce and consume these error values, for precise meaning.
+  enum PreflightStatus {
+    kPreflightSuccess,
+    kPreflightInvalidStatus,
+    // Experimental Access-Control-Allow-External error conditions:

[Mike West, 2017/01/09 09:02:22]

Nit: Can you add a link to https://wicg.github.io/cors-rfc1918/#headers?

[sof, 2017/01/09 09:36:49]

Makes sense, added.

+    kPreflightMissingAllowExternal,
+    kPreflightInvalidAllowExternal,
+  };
+
+  // Enumerating the error conditions that CORS redirect target URL
+  // checks can report, including success.
+  //
+  // See |checkRedirectLocation()| methods and |redirectErrorString()| which
+  // respectively produce and consume these error values, for precise meaning.
+  enum RedirectStatus {
+    kRedirectSuccess,
+    kRedirectDisallowedScheme,
+    kRedirectContainsCredentials,
+  };
+
+  // Perform a CORS access check on the response. Returns |kAccessAllowed| if
+  // access is allowed. Use |accessControlErrorString()| to construct a
+  // user-friendly error message for any of the other (error) conditions.
+  static AccessStatus checkAccess(const ResourceResponse&,
+                                  StoredCredentials,
+                                  const SecurityOrigin*);
+
+  // Perform the required CORS checks on the response to a preflight request.
+  // Returns |kPreflightSuccess| if preflight response was successful.
+  // Use |preflightErrorString()| to construct a user-friendly error message
+  // for any of the other (error) conditions.
+  static PreflightStatus checkPreflight(const ResourceResponse&);
+
+  // Error checking for the currently experimental
+  // Access-Control-Allow-External:
+  // header. Shares error conditions with standard preflight.
+  static PreflightStatus checkExternalPreflight(const ResourceResponse&);
+
+  // Given a redirected-to URL, check if the location is allowed
+  // according to CORS. That is,

[Mike West, 2017/01/09 09:02:22]

Nit: s/,/:/?

[sof, 2017/01/09 09:36:49]

Done.

   // - the URL has a CORS supported scheme and
   // - the URL does not contain the userinfo production.
-  static bool isLegalRedirectLocation(const KURL&, String& errorDescription);
+  //
+  // Returns |kRedirectSuccess| in all other cases. Use
+  // |redirectErrorString()| to construct a user-friendly error
+  // message for any of the error conditions.
+  static RedirectStatus checkRedirectLocation(const KURL&);
+
   static bool handleRedirect(PassRefPtr<SecurityOrigin>,
                              ResourceRequest&,
                              const ResourceResponse&,
                              StoredCredentials,
                              ResourceLoaderOptions&,
                              String&);
+
+  // Stringify errors from CORS access checks, preflight or
+  // redirect checks.
+

[Mike West, 2017/01/09 09:02:22]

Nit: Remove the newline?

[sof, 2017/01/09 09:36:49]

Done.

+  static void accessControlErrorString(StringBuilder&,
+                                       AccessStatus,
+                                       const ResourceResponse&,
+                                       const SecurityOrigin*,
+                                       WebURLRequest::RequestContext);
+  static void preflightErrorString(StringBuilder&,
+                                   PreflightStatus,
+                                   const ResourceResponse&);
+  static void redirectErrorString(StringBuilder&, RedirectStatus, const KURL&);
 };
 
+// TODO: also migrate these into the above static class.
 CORE_EXPORT bool isOnAccessControlResponseHeaderWhitelist(const String&);
 
 CORE_EXPORT ResourceRequest
 createAccessControlPreflightRequest(const ResourceRequest&,
                                     const SecurityOrigin*);
 
-bool passesAccessControlCheck(const ResourceResponse&,
-                              StoredCredentials,
-                              const SecurityOrigin*,
-                              String& errorDescription,
-                              WebURLRequest::RequestContext requestType);
-bool passesPreflightStatusCheck(const ResourceResponse&,
-                                String& errorDescription);
-bool passesExternalPreflightCheck(const ResourceResponse&,
-                                  String& errorDescription);
 CORE_EXPORT void parseAccessControlExposeHeadersAllowList(
     const String& headerValue,
     HTTPHeaderSet&);