From d09798e9f5c43aaa16adaa193876b0de73e788f4 Mon Sep 17 00:00:00 2001
From: Brian Smith <brian@briansmith.org>
Date: Tue, 14 Apr 2015 05:32:29 -1000
Subject: [PATCH] Bug 1154399 - Part 2: Simplify and un-inline OptionalVersion.
 r=keeler

Also fixes the wrong comment. The syntax for version in OCSP and X.509
certs is identical.

--HG--
extra : rebase_source : 744a2998ce8c55a61fbbc1966bc22e4903fa2484
---
 security/pkix/lib/pkixder.cpp | 28 +++++++++++++++++++++++
 security/pkix/lib/pkixder.h   | 42 +++++------------------------------
 2 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/security/pkix/lib/pkixder.cpp b/security/pkix/lib/pkixder.cpp
index 3a02895fa9aa..a87ba99870e7 100644
--- a/security/pkix/lib/pkixder.cpp
+++ b/security/pkix/lib/pkixder.cpp
@@ -577,4 +577,32 @@ IntegralValue(Reader& input, uint8_t tag, /*out*/ uint8_t& value)
 
 } // namespace internal
 
+Result
+OptionalVersion(Reader& input, /*out*/ Version& version)
+{
+  static const uint8_t TAG = CONTEXT_SPECIFIC | CONSTRUCTED | 0;
+  if (!input.Peek(TAG)) {
+    version = Version::v1;
+    return Success;
+  }
+  return Nested(input, TAG, [&version](Reader& value) -> Result {
+    uint8_t integerValue;
+    Result rv = Integer(value, integerValue);
+    if (rv != Success) {
+      return rv;
+    }
+    // XXX(bug 1031093): We shouldn't accept an explicit encoding of v1,
+    // but we do here for compatibility reasons.
+    switch (integerValue) {
+      case static_cast<uint8_t>(Version::v3): version = Version::v3; break;
+      case static_cast<uint8_t>(Version::v2): version = Version::v2; break;
+      case static_cast<uint8_t>(Version::v1): version = Version::v1; break;
+      case static_cast<uint8_t>(Version::v4): version = Version::v4; break;
+      default:
+        return Result::ERROR_BAD_DER;
+    }
+    return Success;
+  });
+}
+
 } } } // namespace mozilla::pkix::der
diff --git a/security/pkix/lib/pkixder.h b/security/pkix/lib/pkixder.h
index d6939c2c10a7..44b77a74d31a 100644
--- a/security/pkix/lib/pkixder.h
+++ b/security/pkix/lib/pkixder.h
@@ -459,43 +459,11 @@ CertificateSerialNumber(Reader& input, /*out*/ Input& value)
 // only supports v1.
 enum class Version { v1 = 0, v2 = 1, v3 = 2, v4 = 3 };
 
-// X.509 Certificate and OCSP ResponseData both use this
-// "[0] EXPLICIT Version DEFAULT <defaultVersion>" construct, but with
-// different default versions.
-inline Result
-OptionalVersion(Reader& input, /*out*/ Version& version)
-{
-  static const uint8_t TAG = CONTEXT_SPECIFIC | CONSTRUCTED | 0;
-  if (!input.Peek(TAG)) {
-    version = Version::v1;
-    return Success;
-  }
-  Reader value;
-  Result rv = ExpectTagAndGetValue(input, TAG, value);
-  if (rv != Success) {
-    return rv;
-  }
-  uint8_t integerValue;
-  rv = Integer(value, integerValue);
-  if (rv != Success) {
-    return rv;
-  }
-  rv = End(value);
-  if (rv != Success) {
-    return rv;
-  }
-  switch (integerValue) {
-    case static_cast<uint8_t>(Version::v3): version = Version::v3; break;
-    case static_cast<uint8_t>(Version::v2): version = Version::v2; break;
-    // XXX(bug 1031093): We shouldn't accept an explicit encoding of v1, but we
-    // do here for compatibility reasons.
-    case static_cast<uint8_t>(Version::v1): version = Version::v1; break;
-    case static_cast<uint8_t>(Version::v4): version = Version::v4; break;
-    default:
-      return Result::ERROR_BAD_DER;
-  }
-  return Success;
-}
+// X.509 Certificate and OCSP ResponseData both use
+// "[0] EXPLICIT Version DEFAULT v1". Although an explicit encoding of v1 is
+// illegal, we support it because some real-world OCSP responses explicitly
+// encode it.
+Result OptionalVersion(Reader& input, /*out*/ Version& version);
 
 template <typename ExtensionHandler>
 inline Result