TS-2939 Metalink: Fix crash when checking the digest of a file that wasn't cacheable
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/fd7365db Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/fd7365db Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/fd7365db Branch: refs/heads/master Commit: fd7365db20cda8d0ef120723f22baed0afb64288 Parents: 74a9a6b Author: Jack Bates <[email protected]> Authored: Wed Jul 16 13:50:56 2014 -0700 Committer: Jack Bates <[email protected]> Committed: Tue Jul 22 08:39:20 2014 -0700 ---------------------------------------------------------------------- CHANGES | 7 +- plugins/experimental/metalink/metalink.cc | 22 ++-- plugins/experimental/metalink/test/location | 4 + plugins/experimental/metalink/test/notCacheable | 104 +++++++++++++++++++ 4 files changed, 129 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fd7365db/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index e5baca6..48c127b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,9 @@ -*- coding: utf-8 -*- Changes with Apache Traffic Server 5.1.0 + *) [TS-2939] Metalink: Fix crash when checking the digest of a file + that wasn't cacheable + *) [TS-2922] Support the new ppc64le platform. Author: Breno Leitao *) [TS-2944] Remove the proxy.config.http.record_tcp_mem_hit configuration variable. @@ -426,7 +429,7 @@ Changes with Apache Traffic Server 5.0.0 *) [TS-2679] background_fetch plugin can use uninitialized cont pointer. - *) [TS-2675] metalink: Fix crash and plug memory leaks. + *) [TS-2675] Metalink: Fix crash and plug memory leaks. Author: Jack Bates <[email protected]> *) [TS-2674] Remove debug printf() from traffic_top. @@ -1468,7 +1471,7 @@ Changes with Apache Traffic Server 3.3.3 (never released) *) [TS-1925] Remove obsolete MMH hash API. - *) [TS-1924] Add metalink plugin documentation. + *) [TS-1924] Add Metalink plugin documentation. Author: Jack Bates <[email protected]> *) [TS-1913] Fix memory issue caused by resolve_logfield_string() http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fd7365db/plugins/experimental/metalink/metalink.cc ---------------------------------------------------------------------- diff --git a/plugins/experimental/metalink/metalink.cc b/plugins/experimental/metalink/metalink.cc index 6461121..6c13df8 100644 --- a/plugins/experimental/metalink/metalink.cc +++ b/plugins/experimental/metalink/metalink.cc @@ -90,6 +90,7 @@ typedef struct { /* Digest header field value index */ int idx; + TSVConn connp; TSIOBuffer cache_bufp; const char *value; @@ -163,7 +164,7 @@ cache_open_write(TSCont contp, void *edata) TSfree(value); - /* Reuse the TSCacheWrite() continuation */ + /* Reentrant! Reuse the TSCacheWrite() continuation. */ TSVConnWrite(data->connp, contp, readerp, nbytes); return 0; @@ -377,7 +378,8 @@ vconn_write_ready(TSCont contp, void */* edata ATS_UNUSED */) /* Determines the Content-Length header (or a chunked response) */ - /* Avoid failed assert "nbytes >= 0" if the response is chunked */ + /* Reentrant! Avoid failed assert "nbytes >= 0" if the response + * is chunked. */ int nbytes = TSVIONBytesGet(input_viop); transform_data->output_viop = TSVConnWrite(output_connp, contp, readerp, nbytes < 0 ? INT64_MAX : nbytes); @@ -471,6 +473,7 @@ vconn_write_ready(TSCont contp, void */* edata ATS_UNUSED */) contp = TSContCreate(write_handler, NULL); TSContDataSet(contp, write_data); + /* Reentrant! */ TSCacheWrite(contp, write_data->key); } @@ -542,13 +545,12 @@ static int cache_open_read(TSCont contp, void *edata) { SendData *data = (SendData *) TSContDataGet(contp); - - TSVConn connp = (TSVConn) edata; + data->connp = (TSVConn) edata; data->cache_bufp = TSIOBufferCreate(); - /* Reuse the TSCacheRead() continuation */ - TSVConnRead(connp, contp, data->cache_bufp, INT64_MAX); + /* Reentrant! Reuse the TSCacheRead() continuation. */ + TSVConnRead(data->connp, contp, data->cache_bufp, INT64_MAX); return 0; } @@ -621,6 +623,8 @@ vconn_read_ready(TSCont contp, void */* edata ATS_UNUSED */) SendData *data = (SendData *) TSContDataGet(contp); TSContDestroy(contp); + TSVConnClose(data->connp); + TSIOBufferReader readerp = TSIOBufferReaderAlloc(data->cache_bufp); TSIOBufferBlock blockp = TSIOBufferReaderStart(readerp); @@ -668,6 +672,10 @@ vconn_read_ready(TSCont contp, void */* edata ATS_UNUSED */) contp = TSContCreate(rewrite_handler, NULL); TSContDataSet(contp, data); + /* Reentrant! (Particularly in case of a cache miss.) + * rewrite_handler() will clean up the TSVConnRead() buffer so be + * sure to close this virtual connection or CacheVC::openReadMain() + * will continue operating on it! */ TSCacheRead(contp, data->key); return 0; @@ -735,6 +743,7 @@ location_handler(TSCont contp, TSEvent event, void */* edata ATS_UNUSED */) contp = TSContCreate(digest_handler, NULL); TSContDataSet(contp, data); + /* Reentrant! */ TSCacheRead(contp, data->key); return 0; @@ -857,6 +866,7 @@ http_send_response_hdr(TSCont contp, void *edata) contp = TSContCreate(location_handler, NULL); TSContDataSet(contp, data); + /* Reentrant! */ TSCacheRead(contp, data->key); return 0; http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fd7365db/plugins/experimental/metalink/test/location ---------------------------------------------------------------------- diff --git a/plugins/experimental/metalink/test/location b/plugins/experimental/metalink/test/location index b1f4710..6e293a6 100755 --- a/plugins/experimental/metalink/test/location +++ b/plugins/experimental/metalink/test/location @@ -39,6 +39,10 @@ class factory(http.HTTPFactory): if target == '/location': + # Satisfy every case of + # proxy.config.http.cache.required_headers + ctx.setHeader('Cache-Control', 'max-age=1') + ctx.write('location') ctx.finish() http://git-wip-us.apache.org/repos/asf/trafficserver/blob/fd7365db/plugins/experimental/metalink/test/notCacheable ---------------------------------------------------------------------- diff --git a/plugins/experimental/metalink/test/notCacheable b/plugins/experimental/metalink/test/notCacheable new file mode 100755 index 0000000..33d8184 --- /dev/null +++ b/plugins/experimental/metalink/test/notCacheable @@ -0,0 +1,104 @@ +#!/usr/bin/env python + +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed +# with this work for additional information regarding copyright +# ownership. The ASF licenses this file to you 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. + +print '''1..1 notCacheable +# The digest of a file that wasn't cacheable doesn't crash the proxy''' + +from twisted.internet import error, protocol, reactor, tcp +from twisted.web import http + +def callback(): + print 'not ok 1 - Why didn\'t the test finish yet?' + + reactor.stop() + +reactor.callLater(1, callback) + +class factory(http.HTTPFactory): + class protocol(http.HTTPChannel): + class requestFactory(http.Request): + def requestReceived(ctx, method, target, version): + + ctx.client = None + ctx.clientproto = version + + if target == '/notCacheable': + + ctx.write('notCacheable') + ctx.finish() + + else: + + ctx.setHeader('Digest', 'SHA-256=BSg5n9c6XBC3jySKsXViB71jhPIoRo3AbCC/gtNlt6k=') + ctx.setHeader('Location', 'http://example.com') + ctx.finish() + +origin = tcp.Port(0, factory()) +origin.startListening() + +print '# Listening on {0}:{1}'.format(*origin.socket.getsockname()) + +class factory(protocol.ClientFactory): + def clientConnectionFailed(ctx, connector, reason): + + print 'Bail out!' + reason.printTraceback() + + reactor.stop() + + class protocol(http.HTTPClient): + def connectionLost(ctx, reason): + try: + reactor.stop() + + except error.ReactorNotRunning: + pass + + else: + print 'not ok 1 - Did the proxy crash? (The client connection closed.)' + + def connectionMade(ctx): + + # A cache MUST NOT store a response to any request, unless: The + # request method is understood by the cache and defined as being + # cacheable, + ctx.transport.write('NOTCACHEABLE {0}:{1}/notCacheable HTTP/1.1\r\n\r\nGET {0}:{1} HTTP/1.1\r\n\r\n'.format(*origin.socket.getsockname())) + + def handleResponsePart(ctx, data): + try: + h, r = data.split('0\r\n\r\n', 1) + + except ValueError: + pass + + else: + + ctx.firstLine = True + ctx.setLineMode(r) + + def handleStatus(ctx, version, status, message): + def handleStatus(version, status, message): + print 'ok 1 - The proxy didn\'t crash (got a response status)' + + reactor.stop() + + ctx.handleStatus = handleStatus + +tcp.Connector('localhost', 8080, factory(), 30, None, reactor).connect() + +reactor.run()
