Address comments

library/common/extensions/filters/http/socket_tag/filter.cc

@@ -24,20 +24,20 @@ Http::FilterHeadersStatus SocketTagFilter::decodeHeaders(Http::RequestHeaderMap&
   uid_t uid;
   uint32_t traffic_stats_tag;
   if (!absl::SimpleAtoi(data.first, &uid) || !absl::SimpleAtoi(data.second, &traffic_stats_tag)) {
+      decoder_callbacks_->sendLocalReply(Http::Code::BadRequest,
+                                         "Invalid socket-tag header.",
+                                         nullptr, absl::nullopt, "");
+      // return Http::FilterDataStatus::StopIterationNoBuffer;
     return Http::FilterHeadersStatus::StopIteration;
   }
 
   auto options = std::make_shared<Network::Socket::Options>();
   options->push_back(std::make_shared<Network::SocketTagSocketOptionImpl>(uid, traffic_stats_tag));
-  callbacks_->addUpstreamSocketOptions(options);
+  decoder_callbacks_->addUpstreamSocketOptions(options);
   request_headers.remove(socket_tag_header);
   return Http::FilterHeadersStatus::Continue;
 }
 
-void SocketTagFilter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) {
-  callbacks_ = &callbacks;
-}
-
 } // namespace SocketTag
 } // namespace HttpFilters
 } // namespace Extensions

library/common/extensions/filters/http/socket_tag/filter.h

@@ -14,17 +14,14 @@ namespace HttpFilters {
 namespace SocketTag {
 
 /**
- * Filter to set upstream socket options based on network conditions.
+ * Filter to set upstream socket tags based on a request header.
+ * See: https://source.android.com/devices/tech/datausage/tags-explained
  */
 class SocketTagFilter final : public Http::PassThroughFilter,
                               public Logger::Loggable<Logger::Id::filter> {
 public:
-  // Http::StreamDecoderFilter
-  void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override;
+  // Http::PassThroughDecoderFilter
   Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& request_headers, bool) override;
-
-private:
-  Http::StreamDecoderFilterCallbacks* callbacks_{};
 };
 
 } // namespace SocketTag

library/common/extensions/filters/http/socket_tag/filter.proto

@@ -2,5 +2,13 @@ syntax = "proto3";
 
 package envoymobile.extensions.filters.http.socket_tag;
 
+
+// Configuration for the socket tagging filer. This filter uses data from the
+// socket-tag request header to apply and Android Socket Tag to the upstream
+// socket.
+// See: https://source.android.com/devices/tech/datausage/tags-explained
+// See: https://developer.android.com/reference/android/net/TrafficStats#setThreadStatsTag(int)
+// See: https://developer.android.com/reference/android/net/TrafficStats#setThreadStatsUid(int)
+// See: https://developer.android.com/reference/android/net/TrafficStats#tagSocket(java.net.Socket)
 message SocketTag {
 }

library/common/network/socket_tag_socket_option_impl.cc

@@ -19,6 +19,11 @@ bool SocketTagSocketOptionImpl::setOption(
     return true;
   }
 
+  // Because socket tagging happens at the socket level, not at the request level,
+  // requests with different socket tags must not use the same socket. As a result
+  // different socket tag socket options must end up in different socket pools.
+  // This happens because different socket tag socket option generate different
+  // hash keys.
   int fd = socket.ioHandle().fdDoNotUse();
   tag_socket(fd, 0, 0);
   tag_socket(fd, uid_, traffic_stats_tag_);
@@ -39,7 +44,10 @@ absl::optional<Socket::Option::Details> SocketTagSocketOptionImpl::getOptionDeta
   static std::string name = "socket_tag";
   Socket::Option::Details details;
   details.name_ = optname_;
-  //  details.value_ = tag_->dataForLogging();
+  std::vector<uint8_t> data;
+  pushScalarToByteVector(uid_, data);
+  pushScalarToByteVector(traffic_stats_tag_, data);
+  details.value_ = std::string(reinterpret_cast<char*>(data.data()), data.size());
   return absl::make_optional(std::move(details));
 }
 

library/common/network/socket_tag_socket_option_impl.h

@@ -27,7 +27,11 @@ class SocketTagSocketOptionImpl : public Network::Socket::Option {
 private:
   const Network::SocketOptionName optname_;
 
+  // Thread stats UID to be applied to the socket.
+  // See: https://developer.android.com/reference/android/net/TrafficStats#setThreadStatsUid(int)
   uid_t uid_;
+  // Thread stats tag to be applied to the socket.
+  // See: https://developer.android.com/reference/android/net/TrafficStats#setThreadStatsTag(int)
   uint32_t traffic_stats_tag_;
 };
 

library/java/org/chromium/net/AndroidNetworkLibrary.java

@@ -210,7 +210,6 @@ private static void tagSocket(int ifd, int uid, int tag) throws IOException {
     if (tag != oldTag) {
       TrafficStats.setThreadStatsTag(tag);
     }
-    // if (uid != TrafficStatsUid.UNSET) {
     if (uid != -1) {
       ThreadStatsUid.set(uid);
     }
@@ -247,7 +246,6 @@ private static void tagSocket(int ifd, int uid, int tag) throws IOException {
     if (tag != oldTag) {
       TrafficStats.setThreadStatsTag(oldTag);
     }
-    //    if (uid != TrafficStatsUid.UNSET) {
     if (uid != -1) {
       ThreadStatsUid.clear();
     }