Hi Graham,
On Tue, Dec 9, 2014 at 5:50 PM, Graham Leggett <minf...@sharp.fm> wrote:
> On 01 Dec 2014, at 7:45 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
>
>> OK, the other way around then (I thought osize was the block size), so
>> it can read more bytes than asked to.
>> This may also be unexpected by the caller...
>>
>> At least for the nonblocking case, I think we should return what we
>> have so far (if any).
>
> Something like this?
Regarding the input filter wrt nonblocking, I'd rather do it this way
(modifications inline) :
+static apr_status_t crypto_in_filter(ap_filter_t *f, apr_bucket_brigade *bb,
+ ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes)
+{
+ apr_bucket *e, *after;
+ apr_status_t rv = APR_SUCCESS;
+ crypto_ctx *ctx = f->ctx;
+
+ if (!ctx->tmp) {
+ ctx->tmp = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
+ }
+
+ /* just get out of the way of things we don't want. */
+ if (mode != AP_MODE_READBYTES) {
+ return ap_get_brigade(f->next, bb, mode, block, readbytes);
+ }
+
+ /* if our buffer is empty, read off the network until the buffer is full */
+ if (APR_BRIGADE_EMPTY(ctx->bb)) {
+ ctx->remaining = ctx->osize;
+ ctx->written = 0;
+
+ while (!ctx->seen_eos && ctx->remaining > 0) {
+ const char *data;
+ apr_size_t size = 0;
+
+ rv = ap_get_brigade(f->next, ctx->tmp, mode, block,
ctx->remaining);
+
+ /* if an error was received, bail out now. If the error is
+ * EAGAIN and we have not yet seen an EOS, we will definitely
+ * be called again, at which point we will send our buffered
+ * data. Instead of sending EAGAIN, some filters return an
+ * empty brigade instead when data is not yet available. In
+ * this case, we drop through and pass buffered data, if any.
+ */
+ if (!APR_STATUS_IS_EAGAIN(rv) && APR_SUCCESS != rv) {
+ return rv;
+ }
Here:
if (APR_STATUS_IS_EAGAIN(rv) || APR_BRIGADE_EMPTY(ctx->tmp)) {
if (APR_BRIGADE_EMPTY(ctx->bb)) {
return rv:
}
break;
}
if (APR_SUCCESS != rv) {
return rv;
}
+
+ for (e = APR_BRIGADE_FIRST(ctx->tmp);
+ e != APR_BRIGADE_SENTINEL(ctx->tmp);
+ e = APR_BUCKET_NEXT(e)) {
+
+ /* if we see an EOS, we are done */
+ if (APR_BUCKET_IS_EOS(e)) {
+
+ /* handle any leftovers */
+ do_crypto(f, NULL, 0, 1);
+ apr_brigade_write(ctx->bb, NULL, NULL,
+ (const char *) ctx->out, ctx->written);
+
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
+ ctx->seen_eos = 1;
+ break;
+ }
+
+ /* flush buckets clear the buffer */
+ if (APR_BUCKET_IS_FLUSH(e)) {
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
+ break;
+ }
+
+ /* pass metadata buckets through */
+ if (APR_BUCKET_IS_METADATA(e)) {
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
+ continue;
+ }
+
+ /* read the bucket in, pack it into the buffer */
+ rv = apr_bucket_read(e, &data, &size, block);
+ if (APR_SUCCESS == rv || APR_STATUS_IS_EAGAIN(rv)) {
And here instead :
rv = apr_bucket_read(e, &data, &size, APR_BLOCK_READ);
if (APR_SUCCESS == rv) {
since nothing should block (likely not a socket or pipe bucket).
Otherwise we'd have to keep this bucket to restart from it on the next call...
+
+ do_crypto(f, (unsigned char *) data, size, 0);
+ if (!ctx->remaining || APR_STATUS_IS_EAGAIN(rv)) {
+ apr_brigade_write(ctx->bb, NULL, NULL,
+ (const char *) ctx->out, ctx->written);
+ }
+
+ apr_bucket_delete(e);
+ }
+ else {
+ return rv;
+ }
+
+ }
+ }
+ }
+
+ /* give the caller the data they asked for from the buffer */
+ apr_brigade_partition(ctx->bb, readbytes, &after);
+ e = APR_BRIGADE_FIRST(ctx->bb);
+ while (e != after) {
+ if (APR_BUCKET_IS_EOS(e)) {
+ /* last bucket read, step out of the way */
+ ap_remove_input_filter(f);
+ }
+ APR_BUCKET_REMOVE(e);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ e = APR_BRIGADE_FIRST(ctx->bb);
+ }
+
+ /* clear the content length */
+ if (!ctx->clength) {
+ ctx->clength = 1;
+ apr_table_unset(f->r->headers_in, "Content-Length");
+ }
+
+ return rv;
+}
Regards,
Yann.
>
> Regards,
> Graham
> —
