You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ro...@apache.org on 2018/05/23 16:41:53 UTC

[incubator-trafficcontrol] branch master updated (cb54909 -> 7061aea)

This is an automated email from the ASF dual-hosted git repository.

rob pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-trafficcontrol.git.


    from cb54909  Clean up
     new a3a7be3  Start of Range Request handler: getfull mode. No error/boundary checking yet
     new 412ada5  int64 for lenghts, and some boundary checks; cleanup
     new bda941f  Add "store_range" mode, and required hook to change the cachekey
     new 7061aea  Implement review comments

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 grove/cache/handler.go            |   4 +
 grove/plugin/README.md            |   2 +
 grove/plugin/plugin.go            |  20 ++++
 grove/plugin/range_req_handler.go | 233 ++++++++++++++++++++++++++++++++++++++
 grove/remap/remap.go              |   1 +
 5 files changed, 260 insertions(+)
 create mode 100644 grove/plugin/range_req_handler.go

-- 
To stop receiving notification emails like this one, please contact
rob@apache.org.

[incubator-trafficcontrol] 02/04: int64 for lenghts, and some boundary checks; cleanup

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rob pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-trafficcontrol.git

commit 412ada5cb8d9345cf86cfa134a77879015998cba
Author: Jan van Doorn <ja...@comcast.com>
AuthorDate: Sat May 12 14:40:02 2018 -0600

    int64 for lenghts, and some boundary checks; cleanup
---
 grove/plugin/range_req_handler.go | 87 +++++++++++++--------------------------
 1 file changed, 28 insertions(+), 59 deletions(-)

diff --git a/grove/plugin/range_req_handler.go b/grove/plugin/range_req_handler.go
index bc21544..321aa3c 100644
--- a/grove/plugin/range_req_handler.go
+++ b/grove/plugin/range_req_handler.go
@@ -1,18 +1,5 @@
 package plugin
 
-import (
-	"encoding/json"
-
-	"crypto/rand"
-	"encoding/hex"
-	"fmt"
-	"github.com/apache/incubator-trafficcontrol/grove/web"
-	"github.com/apache/incubator-trafficcontrol/lib/go-log"
-	"net/http"
-	"strconv"
-	"strings"
-)
-
 /*
    Licensed under the Apache License, Version 2.0 (the "License");
    you may not use this file except in compliance with the License.
@@ -27,9 +14,22 @@ import (
    limitations under the License.
 */
 
+import (
+	"crypto/rand"
+	"encoding/hex"
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"strconv"
+	"strings"
+
+	"github.com/apache/incubator-trafficcontrol/grove/web"
+	"github.com/apache/incubator-trafficcontrol/lib/go-log"
+)
+
 type byteRange struct {
-	Start int
-	End   int
+	Start int64
+	End   int64
 }
 
 type rangeRequestConfig struct {
@@ -50,6 +50,9 @@ func rangeReqHandleLoad(b json.RawMessage) interface{} {
 		log.Errorln("range_rew_handler  loading config, unmarshalling JSON: " + err.Error())
 		return nil
 	}
+	if !(cfg.Mode == "getfull" || cfg.Mode == "patch") {
+		log.Errorf("Unknown mode for range_req_handler plugin: %s", cfg.Mode)
+	}
 	log.Debugf("range_rew_handler: load success: %+v\n", cfg)
 	return &cfg
 }
@@ -62,6 +65,7 @@ func rangeReqHandlerOnRequest(icfg interface{}, d OnRequestData) bool {
 		return false
 	}
 	log.Debugf("Range string is: %s", rHeader)
+	// put the ranges [] in the context so we can use it later
 	byteRanges := parseRangeHeader(rHeader)
 	*d.Context = byteRanges
 	return false
@@ -82,49 +86,14 @@ func rangeReqHandleBeforeParent(icfg interface{}, d BeforeParentRequestData) {
 		return
 	}
 	if cfg.Mode == "getfull" {
-		// getfull means get the whole thing from parent/org, but serve the requested range. Just remove the Range header from the upstream request, and put the range header in the context so we can use it in the response.
+		// getfull means get the whole thing from parent/org, but serve the requested range. Just remove the Range header from the upstream request
 		d.Req.Header.Del("Range")
 	}
 	return
 }
 
-//--
-//> GET /10Mb.txt HTTP/1.1
-//> Host: localhost
-//> Range: bytes=0-1,500000-500009
-//> User-Agent: curl/7.54.0
-//> Accept: */*
-//>
-//< HTTP/1.1 206 Partial Content
-//< Date: Sat, 12 May 2018 14:21:56 GMT
-//< Server: Apache/2.4.29 (Unix)
-//< Last-Modified: Sat, 12 May 2018 14:19:11 GMT
-//< ETag: "a00000-56c02efb675c0"
-//< Accept-Ranges: bytes
-//< Content-Length: 216
-//< Cache-Control: max-age=60, public
-//< Expires: Sat, 12 May 2018 14:22:56 GMT
-//< Content-Type: multipart/byteranges; boundary=4d583e1cee600e82
-//<
-//
-//--4d583e1cee600e82
-//Content-type: text/plain
-//Content-range: bytes 0-1/10485760
-//
-//00
-//--4d583e1cee600e82
-//Content-type: text/plain
-//Content-range: bytes 500000-500009/10485760
-//
-//000500000
-//
-//--4d583e1cee600e82--
-//--
-
-// https://httpwg.org/specs/rfc7230.html#message.body.length
-
 // rangeReqHandleBeforeRespond builds the 206 response
-// assume all the needed ranges have been put in cache before, which is the truth for "getfull" mode which gets the whole object into cache.
+// Assume all the needed ranges have been put in cache before, which is the truth for "getfull" mode which gets the whole object into cache.
 func rangeReqHandleBeforeRespond(icfg interface{}, d BeforeRespondData) {
 	log.Debugf("rangeReqHandleBeforeRespond calling\n")
 	ictx := d.Context
@@ -148,14 +117,14 @@ func rangeReqHandleBeforeRespond(icfg interface{}, d BeforeRespondData) {
 		multipartBoundaryString = hex.EncodeToString(multipartBoundaryBytes)
 		d.Hdr.Set("Content-Type", fmt.Sprintf("multipart/byteranges; boundary=%s", multipartBoundaryString))
 	}
-	totalContentLength, err := strconv.Atoi(d.Hdr.Get("Content-Length"))
+	totalContentLength, err := strconv.ParseInt(d.Hdr.Get("Content-Length"), 10, 64)
 	if err != nil {
 		log.Errorf("Invalid Content-Length header: %v", d.Hdr.Get("Content-Length"))
 	}
 	body := make([]byte, 0)
 	for _, thisRange := range ctx {
-		if thisRange.End == -1 {
-			thisRange.End = totalContentLength
+		if thisRange.End == -1 || thisRange.End >= totalContentLength { // if the end range is "", or too large serve until the end
+			thisRange.End = totalContentLength - 1
 		}
 		log.Debugf("range:%d-%d", thisRange.Start, thisRange.End)
 		if multipartBoundaryString != "" {
@@ -185,9 +154,9 @@ func parseRange(rangeString string) (byteRange, error) {
 	if parts[0] == "" {
 		bRange.Start = 0
 	} else {
-		start, err := strconv.Atoi(parts[0])
+		start, err := strconv.ParseInt(parts[0], 10, 64)
 		if err != nil {
-			log.Errorf("Error converting rangeString \"%\" to numbers", rangeString)
+			log.Errorf("Error converting rangeString start \"%\" to numbers", rangeString)
 			return byteRange{}, err
 		}
 		bRange.Start = start
@@ -195,9 +164,9 @@ func parseRange(rangeString string) (byteRange, error) {
 	if parts[1] == "" {
 		bRange.End = -1 // -1 means till the end
 	} else {
-		end, err := strconv.Atoi(parts[1])
+		end, err := strconv.ParseInt(parts[1], 10, 64)
 		if err != nil {
-			log.Errorf("Error converting rangeString \"%\" to numbers", rangeString)
+			log.Errorf("Error converting rangeString end \"%\" to numbers", rangeString)
 			return byteRange{}, err
 		}
 		bRange.End = end

-- 
To stop receiving notification emails like this one, please contact
rob@apache.org.

[incubator-trafficcontrol] 03/04: Add "store_range" mode, and required hook to change the cachekey

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rob pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-trafficcontrol.git

commit bda941fd62661e8b556fbbf5662b6bb8353e6761
Author: Jan van Doorn <ja...@comcast.com>
AuthorDate: Sun May 13 12:29:17 2018 -0600

    Add "store_range" mode, and required hook to change the cachekey
---
 grove/cache/handler.go            |  4 ++++
 grove/plugin/plugin.go            | 19 +++++++++++++++++
 grove/plugin/range_req_handler.go | 44 +++++++++++++++++++++++++++++++++------
 grove/remap/remap.go              |  1 +
 4 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/grove/cache/handler.go b/grove/cache/handler.go
index 216378c..74b40a5 100644
--- a/grove/cache/handler.go
+++ b/grove/cache/handler.go
@@ -223,6 +223,10 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	log.Debugf("Serve got Cache-Control %+v (reqid %v)\n", reqCacheControl, reqID)
 
 	connectionClose := h.connectionClose || remappingProducer.ConnectionClose()
+
+	beforeCacheLookUpData := plugin.BeforeCacheLookUpData{Req: r, DefaultCacheKey: remappingProducer.CacheKey()}
+	h.plugins.OnBeforeCacheLookup(remappingProducer.PluginCfg(), pluginContext, beforeCacheLookUpData, remappingProducer.OverrideCacheKey)
+
 	cacheKey := remappingProducer.CacheKey()
 	retrier := NewRetrier(h, reqHeader, reqTime, reqCacheControl, remappingProducer, reqID)
 
diff --git a/grove/plugin/plugin.go b/grove/plugin/plugin.go
index ec425c7..32d1280 100644
--- a/grove/plugin/plugin.go
+++ b/grove/plugin/plugin.go
@@ -47,6 +47,7 @@ type Funcs struct {
 	load                LoadFunc
 	startup             StartupFunc
 	onRequest           OnRequestFunc
+	beforeCacheLookUp   BeforeCacheLookupFunc
 	beforeParentRequest BeforeParentRequestFunc
 	beforeRespond       BeforeRespondFunc
 	afterRespond        AfterRespondFunc
@@ -91,6 +92,12 @@ type BeforeRespondData struct {
 	Context   *interface{}
 }
 
+type BeforeCacheLookUpData struct {
+	Req             *http.Request
+	DefaultCacheKey string
+	Context         *interface{}
+}
+
 type AfterRespondData struct {
 	W         http.ResponseWriter
 	Stats     stat.Stats
@@ -105,6 +112,7 @@ type AfterRespondData struct {
 type LoadFunc func(json.RawMessage) interface{}
 type StartupFunc func(icfg interface{}, d StartupData)
 type OnRequestFunc func(icfg interface{}, d OnRequestData) bool
+type BeforeCacheLookupFunc func(icfg interface{}, d BeforeCacheLookUpData, cacheKeyOverRideFunc func(string))
 type BeforeParentRequestFunc func(icfg interface{}, d BeforeParentRequestData)
 type BeforeRespondFunc func(icfg interface{}, d BeforeRespondData)
 type AfterRespondFunc func(icfg interface{}, d AfterRespondData)
@@ -132,6 +140,7 @@ type Plugins interface {
 	LoadFuncs() map[string]LoadFunc
 	OnStartup(cfgs map[string]interface{}, context map[string]*interface{}, d StartupData)
 	OnRequest(cfgs map[string]interface{}, context map[string]*interface{}, d OnRequestData) bool
+	OnBeforeCacheLookup(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeCacheLookUpData, cacheKeyOverrideFunc func(string))
 	OnBeforeParentRequest(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeParentRequestData)
 	OnBeforeRespond(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeRespondData)
 	OnAfterRespond(cfgs map[string]interface{}, context map[string]*interface{}, d AfterRespondData)
@@ -175,6 +184,16 @@ func (ps pluginsSlice) OnRequest(cfgs map[string]interface{}, context map[string
 	return false
 }
 
+func (ps pluginsSlice) OnBeforeCacheLookup(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeCacheLookUpData, cacheKeyOverrideFunc func(string)) {
+	for _, p := range ps {
+		if p.funcs.beforeCacheLookUp == nil {
+			continue
+		}
+		d.Context = context[p.name]
+		p.funcs.beforeCacheLookUp(cfgs[p.name], d, cacheKeyOverrideFunc)
+	}
+}
+
 func (ps pluginsSlice) OnBeforeParentRequest(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeParentRequestData) {
 	for _, p := range ps {
 		if p.funcs.beforeParentRequest == nil {
diff --git a/grove/plugin/range_req_handler.go b/grove/plugin/range_req_handler.go
index 321aa3c..354a6cc 100644
--- a/grove/plugin/range_req_handler.go
+++ b/grove/plugin/range_req_handler.go
@@ -37,7 +37,13 @@ type rangeRequestConfig struct {
 }
 
 func init() {
-	AddPlugin(10000, Funcs{load: rangeReqHandleLoad, onRequest: rangeReqHandlerOnRequest, beforeParentRequest: rangeReqHandleBeforeParent, beforeRespond: rangeReqHandleBeforeRespond})
+	AddPlugin(10000, Funcs{
+		load:                rangeReqHandleLoad,
+		onRequest:           rangeReqHandlerOnRequest,
+		beforeCacheLookUp:   rangeReqHandleBeforeCacheLookup,
+		beforeParentRequest: rangeReqHandleBeforeParent,
+		beforeRespond:       rangeReqHandleBeforeRespond,
+	})
 }
 
 // rangeReqHandleLoad loads the configuration
@@ -50,7 +56,7 @@ func rangeReqHandleLoad(b json.RawMessage) interface{} {
 		log.Errorln("range_rew_handler  loading config, unmarshalling JSON: " + err.Error())
 		return nil
 	}
-	if !(cfg.Mode == "getfull" || cfg.Mode == "patch") {
+	if !(cfg.Mode == "get_full_serve_range" || cfg.Mode == "patch") {
 		log.Errorf("Unknown mode for range_req_handler plugin: %s", cfg.Mode)
 	}
 	log.Debugf("range_rew_handler: load success: %+v\n", cfg)
@@ -71,7 +77,24 @@ func rangeReqHandlerOnRequest(icfg interface{}, d OnRequestData) bool {
 	return false
 }
 
-// rangeReqHandleBeforeParent changes the parent request if needed (mode == getfull)
+func rangeReqHandleBeforeCacheLookup(icfg interface{}, d BeforeCacheLookUpData, overRideFunc func(string)) {
+	cfg, ok := icfg.(*rangeRequestConfig)
+	if !ok {
+		log.Errorf("range_req_handler config '%v' type '%T' expected *rangeRequestConfig\n", icfg, icfg)
+		return
+	}
+	if cfg.Mode == "store_ranges" {
+		sep := "?"
+		if strings.Contains(d.DefaultCacheKey, "?") {
+			sep = "&"
+		}
+		newKey := d.DefaultCacheKey + sep + "grove_range_req_handler_plugin_data=" + d.Req.Header.Get("Range")
+		overRideFunc(newKey)
+		log.Debugf("range_req_handler: store_ranges default key:%s, new key:%s", d.DefaultCacheKey, newKey)
+	}
+}
+
+// rangeReqHandleBeforeParent changes the parent request if needed (mode == get_full_serve_range)
 func rangeReqHandleBeforeParent(icfg interface{}, d BeforeParentRequestData) {
 	log.Debugf("rangeReqHandleBeforeParent calling.")
 	rHeader := d.Req.Header.Get("Range")
@@ -85,15 +108,15 @@ func rangeReqHandleBeforeParent(icfg interface{}, d BeforeParentRequestData) {
 		log.Errorf("range_req_handler config '%v' type '%T' expected *rangeRequestConfig\n", icfg, icfg)
 		return
 	}
-	if cfg.Mode == "getfull" {
-		// getfull means get the whole thing from parent/org, but serve the requested range. Just remove the Range header from the upstream request
+	if cfg.Mode == "get_full_serve_range" {
+		// get_full_serve_range means get the whole thing from parent/org, but serve the requested range. Just remove the Range header from the upstream request
 		d.Req.Header.Del("Range")
 	}
 	return
 }
 
 // rangeReqHandleBeforeRespond builds the 206 response
-// Assume all the needed ranges have been put in cache before, which is the truth for "getfull" mode which gets the whole object into cache.
+// Assume all the needed ranges have been put in cache before, which is the truth for "get_full_serve_range" mode which gets the whole object into cache.
 func rangeReqHandleBeforeRespond(icfg interface{}, d BeforeRespondData) {
 	log.Debugf("rangeReqHandleBeforeRespond calling\n")
 	ictx := d.Context
@@ -105,6 +128,15 @@ func rangeReqHandleBeforeRespond(icfg interface{}, d BeforeRespondData) {
 		return // there was no (valid) range header
 	}
 
+	cfg, ok := icfg.(*rangeRequestConfig)
+	if !ok {
+		log.Errorf("range_req_handler config '%v' type '%T' expected *rangeRequestConfig\n", icfg, icfg)
+		return
+	}
+	if cfg.Mode == "store_ranges" {
+		return // no need to do anything here.
+	}
+
 	multipartBoundaryString := ""
 	originalContentType := d.Hdr.Get("Content-type")
 	*d.Hdr = web.CopyHeader(*d.Hdr) // copy the headers, we don't want to mod the cacheObj
diff --git a/grove/remap/remap.go b/grove/remap/remap.go
index 9793566..b21f279 100644
--- a/grove/remap/remap.go
+++ b/grove/remap/remap.go
@@ -118,6 +118,7 @@ type RemappingProducer struct {
 }
 
 func (p *RemappingProducer) CacheKey() string                  { return p.cacheKey }
+func (p *RemappingProducer) OverrideCacheKey(newKey string)    { p.cacheKey = newKey }
 func (p *RemappingProducer) ConnectionClose() bool             { return p.rule.ConnectionClose }
 func (p *RemappingProducer) Name() string                      { return p.rule.Name }
 func (p *RemappingProducer) DSCP() int                         { return p.rule.DSCP }

-- 
To stop receiving notification emails like this one, please contact
rob@apache.org.

[incubator-trafficcontrol] 01/04: Start of Range Request handler: getfull mode. No error/boundary checking yet

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rob pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-trafficcontrol.git

commit a3a7be3f3003a504c2eed53238cd9e5e37e84435
Author: Jan van Doorn <ja...@comcast.com>
AuthorDate: Sat May 12 13:38:43 2018 -0600

    Start of Range Request handler: getfull mode. No error/boundary checking yet
---
 grove/plugin/range_req_handler.go | 224 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 224 insertions(+)

diff --git a/grove/plugin/range_req_handler.go b/grove/plugin/range_req_handler.go
new file mode 100644
index 0000000..bc21544
--- /dev/null
+++ b/grove/plugin/range_req_handler.go
@@ -0,0 +1,224 @@
+package plugin
+
+import (
+	"encoding/json"
+
+	"crypto/rand"
+	"encoding/hex"
+	"fmt"
+	"github.com/apache/incubator-trafficcontrol/grove/web"
+	"github.com/apache/incubator-trafficcontrol/lib/go-log"
+	"net/http"
+	"strconv"
+	"strings"
+)
+
+/*
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+type byteRange struct {
+	Start int
+	End   int
+}
+
+type rangeRequestConfig struct {
+	Mode string `json::"mode"`
+}
+
+func init() {
+	AddPlugin(10000, Funcs{load: rangeReqHandleLoad, onRequest: rangeReqHandlerOnRequest, beforeParentRequest: rangeReqHandleBeforeParent, beforeRespond: rangeReqHandleBeforeRespond})
+}
+
+// rangeReqHandleLoad loads the configuration
+func rangeReqHandleLoad(b json.RawMessage) interface{} {
+	cfg := rangeRequestConfig{}
+	log.Errorf("rangeReqHandleLoad loading: %s", b)
+
+	err := json.Unmarshal(b, &cfg)
+	if err != nil {
+		log.Errorln("range_rew_handler  loading config, unmarshalling JSON: " + err.Error())
+		return nil
+	}
+	log.Debugf("range_rew_handler: load success: %+v\n", cfg)
+	return &cfg
+}
+
+// rangeReqHandlerOnRequest determines if there is a Range header, and puts the ranges in *d.Context as a []byteRanges
+func rangeReqHandlerOnRequest(icfg interface{}, d OnRequestData) bool {
+	rHeader := d.R.Header.Get("Range")
+	if rHeader == "" {
+		log.Debugf("No Range header found")
+		return false
+	}
+	log.Debugf("Range string is: %s", rHeader)
+	byteRanges := parseRangeHeader(rHeader)
+	*d.Context = byteRanges
+	return false
+}
+
+// rangeReqHandleBeforeParent changes the parent request if needed (mode == getfull)
+func rangeReqHandleBeforeParent(icfg interface{}, d BeforeParentRequestData) {
+	log.Debugf("rangeReqHandleBeforeParent calling.")
+	rHeader := d.Req.Header.Get("Range")
+	if rHeader == "" {
+		log.Debugf("No Range header found")
+		return
+	}
+	log.Debugf("Range string is: %s", rHeader)
+	cfg, ok := icfg.(*rangeRequestConfig)
+	if !ok {
+		log.Errorf("range_req_handler config '%v' type '%T' expected *rangeRequestConfig\n", icfg, icfg)
+		return
+	}
+	if cfg.Mode == "getfull" {
+		// getfull means get the whole thing from parent/org, but serve the requested range. Just remove the Range header from the upstream request, and put the range header in the context so we can use it in the response.
+		d.Req.Header.Del("Range")
+	}
+	return
+}
+
+//--
+//> GET /10Mb.txt HTTP/1.1
+//> Host: localhost
+//> Range: bytes=0-1,500000-500009
+//> User-Agent: curl/7.54.0
+//> Accept: */*
+//>
+//< HTTP/1.1 206 Partial Content
+//< Date: Sat, 12 May 2018 14:21:56 GMT
+//< Server: Apache/2.4.29 (Unix)
+//< Last-Modified: Sat, 12 May 2018 14:19:11 GMT
+//< ETag: "a00000-56c02efb675c0"
+//< Accept-Ranges: bytes
+//< Content-Length: 216
+//< Cache-Control: max-age=60, public
+//< Expires: Sat, 12 May 2018 14:22:56 GMT
+//< Content-Type: multipart/byteranges; boundary=4d583e1cee600e82
+//<
+//
+//--4d583e1cee600e82
+//Content-type: text/plain
+//Content-range: bytes 0-1/10485760
+//
+//00
+//--4d583e1cee600e82
+//Content-type: text/plain
+//Content-range: bytes 500000-500009/10485760
+//
+//000500000
+//
+//--4d583e1cee600e82--
+//--
+
+// https://httpwg.org/specs/rfc7230.html#message.body.length
+
+// rangeReqHandleBeforeRespond builds the 206 response
+// assume all the needed ranges have been put in cache before, which is the truth for "getfull" mode which gets the whole object into cache.
+func rangeReqHandleBeforeRespond(icfg interface{}, d BeforeRespondData) {
+	log.Debugf("rangeReqHandleBeforeRespond calling\n")
+	ictx := d.Context
+	ctx, ok := (*ictx).([]byteRange)
+	if !ok {
+		log.Errorf("Invalid context: %v", ictx)
+	}
+	if len(ctx) == 0 {
+		return // there was no (valid) range header
+	}
+
+	multipartBoundaryString := ""
+	originalContentType := d.Hdr.Get("Content-type")
+	*d.Hdr = web.CopyHeader(*d.Hdr) // copy the headers, we don't want to mod the cacheObj
+	if len(ctx) > 1 {
+		//multipart = true
+		multipartBoundaryBytes := make([]byte, 8)
+		if _, err := rand.Read(multipartBoundaryBytes); err != nil {
+			log.Errorf("Error with rand.Read: %v", err)
+		}
+		multipartBoundaryString = hex.EncodeToString(multipartBoundaryBytes)
+		d.Hdr.Set("Content-Type", fmt.Sprintf("multipart/byteranges; boundary=%s", multipartBoundaryString))
+	}
+	totalContentLength, err := strconv.Atoi(d.Hdr.Get("Content-Length"))
+	if err != nil {
+		log.Errorf("Invalid Content-Length header: %v", d.Hdr.Get("Content-Length"))
+	}
+	body := make([]byte, 0)
+	for _, thisRange := range ctx {
+		if thisRange.End == -1 {
+			thisRange.End = totalContentLength
+		}
+		log.Debugf("range:%d-%d", thisRange.Start, thisRange.End)
+		if multipartBoundaryString != "" {
+			body = append(body, []byte(fmt.Sprintf("\r\n--%s\r\n", multipartBoundaryString))...)
+			body = append(body, []byte(fmt.Sprintf("Content-type: %s\r\n", originalContentType))...)
+			body = append(body, []byte(fmt.Sprintf("Content-range: bytes %d-%d/%d\r\n\r\n", thisRange.Start, thisRange.End, totalContentLength))...)
+		} else {
+			byteRangeString := fmt.Sprintf("bytes %d-%d/%d", thisRange.Start, thisRange.End, totalContentLength)
+			d.Hdr.Add("Content-Range", byteRangeString)
+		}
+		bSlice := (*d.Body)[thisRange.Start : thisRange.End+1]
+		body = append(body, bSlice...)
+	}
+	if multipartBoundaryString != "" {
+		body = append(body, []byte(fmt.Sprintf("\r\n--%s--\r\n", multipartBoundaryString))...)
+	}
+	d.Hdr.Set("Content-Length", fmt.Sprintf("%d", len(body)))
+	*d.Body = body
+	*d.Code = http.StatusPartialContent
+	return
+}
+
+func parseRange(rangeString string) (byteRange, error) {
+	parts := strings.Split(rangeString, "-")
+
+	var bRange byteRange
+	if parts[0] == "" {
+		bRange.Start = 0
+	} else {
+		start, err := strconv.Atoi(parts[0])
+		if err != nil {
+			log.Errorf("Error converting rangeString \"%\" to numbers", rangeString)
+			return byteRange{}, err
+		}
+		bRange.Start = start
+	}
+	if parts[1] == "" {
+		bRange.End = -1 // -1 means till the end
+	} else {
+		end, err := strconv.Atoi(parts[1])
+		if err != nil {
+			log.Errorf("Error converting rangeString \"%\" to numbers", rangeString)
+			return byteRange{}, err
+		}
+		bRange.End = end
+	}
+	return bRange, nil
+}
+
+func parseRangeHeader(rHdrVal string) []byteRange {
+	byteRanges := make([]byteRange, 0)
+	rangeStringParts := strings.Split(rHdrVal, "=")
+	if rangeStringParts[0] != "bytes" {
+		log.Errorf("Not a valid Range type: \"%s\"", rangeStringParts[0])
+	}
+
+	for _, thisRangeString := range strings.Split(rangeStringParts[1], ",") {
+		log.Debugf("bRangeStr: %s", thisRangeString)
+		thisRange, err := parseRange(thisRangeString)
+		if err != nil {
+			return nil
+		}
+		byteRanges = append(byteRanges, thisRange)
+	}
+	return byteRanges
+}

-- 
To stop receiving notification emails like this one, please contact
rob@apache.org.

[incubator-trafficcontrol] 04/04: Implement review comments

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rob pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-trafficcontrol.git

commit 7061aea8b28eb9b854b6a0ec19d330780c12cf17
Author: Jan van Doorn <jv...@knutsel.com>
AuthorDate: Fri May 18 10:38:39 2018 -0600

    Implement review comments
---
 grove/cache/handler.go            |  4 +--
 grove/plugin/README.md            |  2 ++
 grove/plugin/plugin.go            | 15 +++++----
 grove/plugin/range_req_handler.go | 70 ++++++++++++++++++++++-----------------
 4 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/grove/cache/handler.go b/grove/cache/handler.go
index 74b40a5..0b4e15e 100644
--- a/grove/cache/handler.go
+++ b/grove/cache/handler.go
@@ -224,8 +224,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	connectionClose := h.connectionClose || remappingProducer.ConnectionClose()
 
-	beforeCacheLookUpData := plugin.BeforeCacheLookUpData{Req: r, DefaultCacheKey: remappingProducer.CacheKey()}
-	h.plugins.OnBeforeCacheLookup(remappingProducer.PluginCfg(), pluginContext, beforeCacheLookUpData, remappingProducer.OverrideCacheKey)
+	beforeCacheLookUpData := plugin.BeforeCacheLookUpData{Req: r, DefaultCacheKey: remappingProducer.CacheKey(), CacheKeyOverrideFunc: remappingProducer.OverrideCacheKey}
+	h.plugins.OnBeforeCacheLookup(remappingProducer.PluginCfg(), pluginContext, beforeCacheLookUpData)
 
 	cacheKey := remappingProducer.CacheKey()
 	retrier := NewRetrier(h, reqHeader, reqTime, reqCacheControl, remappingProducer, reqID)
diff --git a/grove/plugin/README.md b/grove/plugin/README.md
index 8dec9be..ec390d7 100644
--- a/grove/plugin/README.md
+++ b/grove/plugin/README.md
@@ -31,6 +31,8 @@ The `Funcs` object contains functions for each hook, as well as a load function
 
 * `onRequest` is called immediately when a request is received. It returns a boolean indicating whether to stop processing. Examples are IP blocking, or serving custom endpoints for statistics or to invalidate a cache entry.
 
+* `beforeCacheLookUp` is called immedidiately before looking the object up in the cache. It can be used to modify the cacheKey to be used to for this object using the passed `CacheKeyOverrideFunc` func. Once set using that function Grove will keep using that cacheKey throughout the life of the object in the cache.
+
 * `beforeParentRequest` is called immediately before making a request to a parent. It may manipulate the request being made to the parent. Examples are removing headers in the client request such as `Range`.
 
 * `beforeRespond` is called immediately before responding to a client. It may manipulate the code, headers, and body being returned. Examples are header modifications, or handling if-modified-since requests.
diff --git a/grove/plugin/plugin.go b/grove/plugin/plugin.go
index 32d1280..140c4e4 100644
--- a/grove/plugin/plugin.go
+++ b/grove/plugin/plugin.go
@@ -93,9 +93,10 @@ type BeforeRespondData struct {
 }
 
 type BeforeCacheLookUpData struct {
-	Req             *http.Request
-	DefaultCacheKey string
-	Context         *interface{}
+	Req                  *http.Request
+	CacheKeyOverrideFunc func(string)
+	DefaultCacheKey      string
+	Context              *interface{}
 }
 
 type AfterRespondData struct {
@@ -112,7 +113,7 @@ type AfterRespondData struct {
 type LoadFunc func(json.RawMessage) interface{}
 type StartupFunc func(icfg interface{}, d StartupData)
 type OnRequestFunc func(icfg interface{}, d OnRequestData) bool
-type BeforeCacheLookupFunc func(icfg interface{}, d BeforeCacheLookUpData, cacheKeyOverRideFunc func(string))
+type BeforeCacheLookupFunc func(icfg interface{}, d BeforeCacheLookUpData)
 type BeforeParentRequestFunc func(icfg interface{}, d BeforeParentRequestData)
 type BeforeRespondFunc func(icfg interface{}, d BeforeRespondData)
 type AfterRespondFunc func(icfg interface{}, d AfterRespondData)
@@ -140,7 +141,7 @@ type Plugins interface {
 	LoadFuncs() map[string]LoadFunc
 	OnStartup(cfgs map[string]interface{}, context map[string]*interface{}, d StartupData)
 	OnRequest(cfgs map[string]interface{}, context map[string]*interface{}, d OnRequestData) bool
-	OnBeforeCacheLookup(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeCacheLookUpData, cacheKeyOverrideFunc func(string))
+	OnBeforeCacheLookup(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeCacheLookUpData)
 	OnBeforeParentRequest(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeParentRequestData)
 	OnBeforeRespond(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeRespondData)
 	OnAfterRespond(cfgs map[string]interface{}, context map[string]*interface{}, d AfterRespondData)
@@ -184,13 +185,13 @@ func (ps pluginsSlice) OnRequest(cfgs map[string]interface{}, context map[string
 	return false
 }
 
-func (ps pluginsSlice) OnBeforeCacheLookup(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeCacheLookUpData, cacheKeyOverrideFunc func(string)) {
+func (ps pluginsSlice) OnBeforeCacheLookup(cfgs map[string]interface{}, context map[string]*interface{}, d BeforeCacheLookUpData) {
 	for _, p := range ps {
 		if p.funcs.beforeCacheLookUp == nil {
 			continue
 		}
 		d.Context = context[p.name]
-		p.funcs.beforeCacheLookUp(cfgs[p.name], d, cacheKeyOverrideFunc)
+		p.funcs.beforeCacheLookUp(cfgs[p.name], d)
 	}
 }
 
diff --git a/grove/plugin/range_req_handler.go b/grove/plugin/range_req_handler.go
index 354a6cc..0bd8e0c 100644
--- a/grove/plugin/range_req_handler.go
+++ b/grove/plugin/range_req_handler.go
@@ -16,7 +16,6 @@ package plugin
 
 import (
 	"crypto/rand"
-	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -33,7 +32,8 @@ type byteRange struct {
 }
 
 type rangeRequestConfig struct {
-	Mode string `json::"mode"`
+	Mode              string `json:"mode"`
+	MultiPartBoundary string // not in the json
 }
 
 func init() {
@@ -49,7 +49,7 @@ func init() {
 // rangeReqHandleLoad loads the configuration
 func rangeReqHandleLoad(b json.RawMessage) interface{} {
 	cfg := rangeRequestConfig{}
-	log.Errorf("rangeReqHandleLoad loading: %s", b)
+	log.Errorf("rangeReqHandleLoad loading: %s\n", b)
 
 	err := json.Unmarshal(b, &cfg)
 	if err != nil {
@@ -57,8 +57,16 @@ func rangeReqHandleLoad(b json.RawMessage) interface{} {
 		return nil
 	}
 	if !(cfg.Mode == "get_full_serve_range" || cfg.Mode == "patch") {
-		log.Errorf("Unknown mode for range_req_handler plugin: %s", cfg.Mode)
+		log.Errorf("Unknown mode for range_req_handler plugin: %s\n", cfg.Mode)
 	}
+
+	multipartBoundaryBytes := make([]byte, 16)
+	if _, err := rand.Read(multipartBoundaryBytes); err != nil {
+		log.Errorf("Error with rand.Read: %v\n", err)
+	}
+	// Use UUID format
+	cfg.MultiPartBoundary = fmt.Sprintf("%x-%x-%x-%x-%x", multipartBoundaryBytes[0:4], multipartBoundaryBytes[4:6], multipartBoundaryBytes[6:8], multipartBoundaryBytes[8:10], multipartBoundaryBytes[10:])
+
 	log.Debugf("range_rew_handler: load success: %+v\n", cfg)
 	return &cfg
 }
@@ -67,17 +75,18 @@ func rangeReqHandleLoad(b json.RawMessage) interface{} {
 func rangeReqHandlerOnRequest(icfg interface{}, d OnRequestData) bool {
 	rHeader := d.R.Header.Get("Range")
 	if rHeader == "" {
-		log.Debugf("No Range header found")
+		log.Debugf("No Range header found\n")
 		return false
 	}
-	log.Debugf("Range string is: %s", rHeader)
+	log.Debugf("Range string is: %s\n", rHeader)
 	// put the ranges [] in the context so we can use it later
 	byteRanges := parseRangeHeader(rHeader)
 	*d.Context = byteRanges
 	return false
 }
 
-func rangeReqHandleBeforeCacheLookup(icfg interface{}, d BeforeCacheLookUpData, overRideFunc func(string)) {
+// rangeReqHandleBeforeCacheLookup is used to override the cacheKey when in store_ranges mode.
+func rangeReqHandleBeforeCacheLookup(icfg interface{}, d BeforeCacheLookUpData) {
 	cfg, ok := icfg.(*rangeRequestConfig)
 	if !ok {
 		log.Errorf("range_req_handler config '%v' type '%T' expected *rangeRequestConfig\n", icfg, icfg)
@@ -89,8 +98,8 @@ func rangeReqHandleBeforeCacheLookup(icfg interface{}, d BeforeCacheLookUpData,
 			sep = "&"
 		}
 		newKey := d.DefaultCacheKey + sep + "grove_range_req_handler_plugin_data=" + d.Req.Header.Get("Range")
-		overRideFunc(newKey)
-		log.Debugf("range_req_handler: store_ranges default key:%s, new key:%s", d.DefaultCacheKey, newKey)
+		d.CacheKeyOverrideFunc(newKey)
+		log.Debugf("range_req_handler: store_ranges default key:%s, new key:%s\n", d.DefaultCacheKey, newKey)
 	}
 }
 
@@ -99,10 +108,10 @@ func rangeReqHandleBeforeParent(icfg interface{}, d BeforeParentRequestData) {
 	log.Debugf("rangeReqHandleBeforeParent calling.")
 	rHeader := d.Req.Header.Get("Range")
 	if rHeader == "" {
-		log.Debugf("No Range header found")
+		log.Debugln("No Range header found")
 		return
 	}
-	log.Debugf("Range string is: %s", rHeader)
+	log.Debugf("Range string is: %s\n", rHeader)
 	cfg, ok := icfg.(*rangeRequestConfig)
 	if !ok {
 		log.Errorf("range_req_handler config '%v' type '%T' expected *rangeRequestConfig\n", icfg, icfg)
@@ -117,12 +126,13 @@ func rangeReqHandleBeforeParent(icfg interface{}, d BeforeParentRequestData) {
 
 // rangeReqHandleBeforeRespond builds the 206 response
 // Assume all the needed ranges have been put in cache before, which is the truth for "get_full_serve_range" mode which gets the whole object into cache.
+// If mode == store_ranges, do nothing, we just return the object stored-as is
 func rangeReqHandleBeforeRespond(icfg interface{}, d BeforeRespondData) {
 	log.Debugf("rangeReqHandleBeforeRespond calling\n")
 	ictx := d.Context
 	ctx, ok := (*ictx).([]byteRange)
 	if !ok {
-		log.Errorf("Invalid context: %v", ictx)
+		log.Errorf("Invalid context: %v\n", ictx)
 	}
 	if len(ctx) == 0 {
 		return // there was no (valid) range header
@@ -137,35 +147,34 @@ func rangeReqHandleBeforeRespond(icfg interface{}, d BeforeRespondData) {
 		return // no need to do anything here.
 	}
 
-	multipartBoundaryString := ""
+	// mode != store_ranges
+	multipartBoundaryString := cfg.MultiPartBoundary
+	multipart := false
 	originalContentType := d.Hdr.Get("Content-type")
 	*d.Hdr = web.CopyHeader(*d.Hdr) // copy the headers, we don't want to mod the cacheObj
 	if len(ctx) > 1 {
-		//multipart = true
-		multipartBoundaryBytes := make([]byte, 8)
-		if _, err := rand.Read(multipartBoundaryBytes); err != nil {
-			log.Errorf("Error with rand.Read: %v", err)
-		}
-		multipartBoundaryString = hex.EncodeToString(multipartBoundaryBytes)
+		multipart = true
+		multipartBoundaryString = cfg.MultiPartBoundary
 		d.Hdr.Set("Content-Type", fmt.Sprintf("multipart/byteranges; boundary=%s", multipartBoundaryString))
 	}
 	totalContentLength, err := strconv.ParseInt(d.Hdr.Get("Content-Length"), 10, 64)
 	if err != nil {
-		log.Errorf("Invalid Content-Length header: %v", d.Hdr.Get("Content-Length"))
+		log.Errorf("Invalid Content-Length header: %v\n", d.Hdr.Get("Content-Length"))
 	}
 	body := make([]byte, 0)
 	for _, thisRange := range ctx {
 		if thisRange.End == -1 || thisRange.End >= totalContentLength { // if the end range is "", or too large serve until the end
 			thisRange.End = totalContentLength - 1
 		}
-		log.Debugf("range:%d-%d", thisRange.Start, thisRange.End)
-		if multipartBoundaryString != "" {
-			body = append(body, []byte(fmt.Sprintf("\r\n--%s\r\n", multipartBoundaryString))...)
-			body = append(body, []byte(fmt.Sprintf("Content-type: %s\r\n", originalContentType))...)
-			body = append(body, []byte(fmt.Sprintf("Content-range: bytes %d-%d/%d\r\n\r\n", thisRange.Start, thisRange.End, totalContentLength))...)
+
+		rangeString := "bytes " + strconv.FormatInt(thisRange.Start, 10) + "-" + strconv.FormatInt(thisRange.End, 10) + "\r\n\r\n"
+		log.Debugf("range:%d-%d\n", thisRange.Start, thisRange.End)
+		if multipart {
+			body = append(body, []byte("\r\n--"+multipartBoundaryString+"\r\n")...)
+			body = append(body, []byte("Content-type: "+originalContentType+"%s\r\n")...)
+			body = append(body, []byte("Content-range: "+rangeString)...)
 		} else {
-			byteRangeString := fmt.Sprintf("bytes %d-%d/%d", thisRange.Start, thisRange.End, totalContentLength)
-			d.Hdr.Add("Content-Range", byteRangeString)
+			d.Hdr.Add("Content-Range", rangeString+"/"+strconv.FormatInt(totalContentLength, 10))
 		}
 		bSlice := (*d.Body)[thisRange.Start : thisRange.End+1]
 		body = append(body, bSlice...)
@@ -188,7 +197,7 @@ func parseRange(rangeString string) (byteRange, error) {
 	} else {
 		start, err := strconv.ParseInt(parts[0], 10, 64)
 		if err != nil {
-			log.Errorf("Error converting rangeString start \"%\" to numbers", rangeString)
+			log.Errorf("Error converting rangeString start \"%\" to numbers\n", rangeString)
 			return byteRange{}, err
 		}
 		bRange.Start = start
@@ -198,7 +207,7 @@ func parseRange(rangeString string) (byteRange, error) {
 	} else {
 		end, err := strconv.ParseInt(parts[1], 10, 64)
 		if err != nil {
-			log.Errorf("Error converting rangeString end \"%\" to numbers", rangeString)
+			log.Errorf("Error converting rangeString end \"%\" to numbers\n", rangeString)
 			return byteRange{}, err
 		}
 		bRange.End = end
@@ -210,11 +219,10 @@ func parseRangeHeader(rHdrVal string) []byteRange {
 	byteRanges := make([]byteRange, 0)
 	rangeStringParts := strings.Split(rHdrVal, "=")
 	if rangeStringParts[0] != "bytes" {
-		log.Errorf("Not a valid Range type: \"%s\"", rangeStringParts[0])
+		log.Errorf("Not a valid Range type: \"%s\"\n", rangeStringParts[0])
 	}
 
 	for _, thisRangeString := range strings.Split(rangeStringParts[1], ",") {
-		log.Debugf("bRangeStr: %s", thisRangeString)
 		thisRange, err := parseRange(thisRangeString)
 		if err != nil {
 			return nil

-- 
To stop receiving notification emails like this one, please contact
rob@apache.org.