poco icon indicating copy to clipboard operation
poco copied to clipboard

Function goes out of scope and ApacheConnector loses Content-Type before HTTP Response be sent

Open edson-a-soares opened this issue 7 years ago • 4 comments

ApacheConnector does not set Content-Type correctly into the HTTP Response. It seems like the function ApacheRequestRec::setContentType(...) goes out of scope before HTTP Response is sent. Therefore, either nothing or some weird character remains as Content-Type. Like this, for example: Content-type: �. As compatibility with the standards previous to cxx_11 is important, two solutions must be made.

A solution using std::unique_ptr would be easier to do, something like this: auto contentType = std::make_unique<char[]>(mediaType.length()+1); td::strcpy(contentType.get(), mediaType.c_str()); ap_set_content_type(_pRec, contentType.release());

The problem seems to be the part o the solution compatible with previous cxx_11 compilers. As std::auto_ptr does not support char * by default, an Adapter could be made.

template <class Tp>
class ArrayReleaseAdapter {
public:
    ArrayReleaseAdapter(Tp* p) : p_(p) {}
    ~ArrayReleaseAdapter() { delete[] p_; }
    Tp* get() const throw() { return p_; }
    Tp* release() throw() {
        Tp* _tmp = p_;
        p_ = 0;
        return _tmp;
    }

private:
    Tp* p_;
};

std::auto_ptr<ArrayReleaseAdapter<char>> contentType(new ArrayReleaseAdapter<char>(
    new char[mediaType.length()+1]
));
std::strcpy(contentType.get()->get(), mediaType.c_str());
ap_set_content_type(_pRec, contentType.release()->release());

But, it is a quite a bit of code for what seems to be a simple problem, so I thought I could use some help before pushing anything to the repository.

Tools Poco 1.9.0 GCC6/G++6 Debian 9/Ubuntu 16.04 LTS

edson-a-soares avatar Aug 03 '18 20:08 edson-a-soares

I'm not understanding the "function goes out of scope" part - did you mean that mediaType goes out of scope after setContentType() completes but before the ApacheRequesRec::_pRec is used at a later point?

I'm also not familiar with the overall functionality, does ap_set_content_type() create an internal copy of the string?

If not, then there should be a std::string ApacheRequesRec::_contentType member, where you keep the copy to make sure it survives until use time. There may be other things in that class that suffer from the same problem.

If mediaType can somehow go out of scope during the setContentType() call, then the only proper solution I can think of is to change the signature to setContentType(Poco::SharedPtr<std::string>), but that probably complicates things significantly by propagating change to the rest of the framework.

aleks-f avatar Aug 04 '18 01:08 aleks-f

Yes, mediaType goes out of scope when setContentType() complete but before ApacheRequesRec::_pRec is used at a later point. Yet, even a copy of mediaType inside setContentType(...) goes out of scope before ApacheRequesRec::_pRec is used.

ap_set_content_type(...) does not keep a copy of the string. Here is its implementation:

AP_DECLARE(void) ap_set_content_type(request_rec *r, const char *ct)     
{    
    if (!ct) {    
        r->content_type = NULL;    
    }    
    else if (!r->content_type || strcmp(r->content_type, ct)) {    
        r->content_type = ct;    
    }    
}    

I tried to use a member variable like std::string ApacheRequesRec::_contentType. The function setContentType(...) would be something like:

void ApacheRequestRec::setContentType(const std::string& mediaType)
{
    contentType = mediaType;
    ap_set_content_type(_pRec, contentType.c_str());
}

Unfortunately, ApacheRequesRec::_pRec is not used before the member variable goes out of scope.
Poco::SharedPtr looked the solution to me too. It would be a framework native solution. No need to use deprecated std::auto_ptr and its ugly adapter. So, I tried a solution with Poco::SharedPtr<char> and Poco::SharedPtr<std::string>. Something like this:

#include "Poco/SharedPtr.h"

char* contentType = new char[mediaType.length()+1];
Poco::SharedPtr< char, Poco::ReferenceCounter, Poco::ReleaseArrayPolicy<char> > tmp(contentType);
std::strcpy(tmp, mediaType.c_str());
ap_set_content_type(_pRec, tmp);

and the one you proposed, changing the signature of the method:

void ApacheRequestRec::setContentType(Poco::SharedPtr<std::string> mediaType)
{
    ap_set_content_type(_pRec, mediaType.get()->c_str());
}

Sadly, none of the two forementioned options worked. The result of the Content-Type still is a random character like: �. By now, only that ugly std::auto_ptr and its adapter seems to work. I will keep working on it until a solution comes out. Any news, I'll post it here before pushing.

edson-a-soares avatar Aug 04 '18 19:08 edson-a-soares

It sounds like the whole ApacheRequestRec instance is gone by the time it is used by the Apache APIs. So, in addition to having the _contentType member, I suggest you look into that direction and make sure ApacheRequestRec survives long enough.

aleks-f avatar Aug 05 '18 17:08 aleks-f

I thought ApacheRequestRec instance could be the problem. However, HTTP Status Code and Headers are being sent correctly with no problem whatsoever. The only problem seems to be content_type. Surely I will look in the direction you suggested.

edson-a-soares avatar Aug 06 '18 18:08 edson-a-soares

This issue was closed because it has been inactive for 60 days since being marked as stale.

github-actions[bot] avatar Aug 21 '22 03:08 github-actions[bot]