Skip to content

Commit

Permalink
HTTP: Add partial handling of OPTIONS and framework for 405 errors
Browse files Browse the repository at this point in the history 
This doesn't entirely work because some of the server extensions are
ambigious, we can't know which ones apply without actually trying to
handle the request, which isn't optimal when we only want to return a
two-line header. On possibility is to eliminate the ambiguity, move
the upnp code to it's own sub-url e.g. /upnp/.

A possible upside to proper 405/OPTIONS support is that the services
code could indicate whether GET or POST are allowed for each endpoint
using HTTP protocol as it was intended. There are some who advocate
for proper use of OPTIONS with REST apis and it costs us nothing to
offer this capability.
  • Loading branch information
@stuartm
stuartm committed Feb 28, 2015
1 parent b76248a commit 373e52c
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 20 deletions.
1 change: 1 addition & 0 deletions mythtv/libs/libmythupnp/eventing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Eventing::Eventing(const QString &sExtensionName,
m_pInitializeSubscriber(NULL)
{
m_sEventMethodName.detach();
m_nSupportedMethods |= (RequestTypeSubscribe | RequestTypeUnsubscribe);
}

/////////////////////////////////////////////////////////////////////////////
Expand Down
16 changes: 11 additions & 5 deletions mythtv/libs/libmythupnp/httprequest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,17 @@ HTTPRequest::HTTPRequest() : m_procReqLineExp ( "[ \r\n][ \r\n]*" ),

RequestType HTTPRequest::SetRequestType( const QString &sType )
{
// HTTP
if (sType == "GET" ) return( m_eType = RequestTypeGet );
if (sType == "HEAD" ) return( m_eType = RequestTypeHead );
if (sType == "POST" ) return( m_eType = RequestTypePost );
if (sType == "M-SEARCH" ) return( m_eType = RequestTypeMSearch );
if (sType == "OPTIONS" ) return( m_eType = RequestTypeOptions );

// UPnP
if (sType == "M-SEARCH" ) return( m_eType = RequestTypeMSearch );
if (sType == "NOTIFY" ) return( m_eType = RequestTypeNotify );
if (sType == "SUBSCRIBE" ) return( m_eType = RequestTypeSubscribe );
if (sType == "UNSUBSCRIBE") return( m_eType = RequestTypeUnsubscribe );
if (sType == "NOTIFY" ) return( m_eType = RequestTypeNotify );

if (sType.startsWith( QString("HTTP/") )) return( m_eType = RequestTypeResponse );

Expand Down Expand Up @@ -2276,18 +2279,21 @@ QString HTTPRequest::GetRequestType( ) const
case RequestTypePost :
type = "POST";
break;
case RequestTypeOptions:
type = "OPTIONS";
break;
case RequestTypeMSearch:
type = "M-SEARCH";
break;
case RequestTypeNotify:
type = "NOTIFY";
break;
case RequestTypeSubscribe :
type = "SUBSCRIBE";
break;
case RequestTypeUnsubscribe :
type = "UNSUBSCRIBE";
break;
case RequestTypeNotify:
type = "NOTIFY";
break;
}

return type;
Expand Down
22 changes: 14 additions & 8 deletions mythtv/libs/libmythupnp/httprequest.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,25 @@
// Typedefs / Defines
/////////////////////////////////////////////////////////////////////////////

// SECURITY: We intentionally want to ignore TRACE requests because of XST attacks
typedef enum
{
RequestTypeUnknown = 0x0000,
// HTTP 1.1
RequestTypeGet = 0x0001,
RequestTypeHead = 0x0002,
RequestTypePost = 0x0004,
RequestTypeMSearch = 0x0008,
RequestTypeSubscribe = 0x0010,
RequestTypeUnsubscribe = 0x0020,
RequestTypeNotify = 0x0040,
RequestTypeResponse = 0x0080
// RequestTypePut = 0x0008,
// RequestTypeDelete = 0x0010,
// RequestTypeConnect = 0x0020,
RequestTypeOptions = 0x0040,
// RequestTypeTrace = 0x0080, // SECURITY: XST attack risk, shouldn't include cookies or other sensitive info
// UPnP
RequestTypeMSearch = 0x0100,
RequestTypeSubscribe = 0x0200,
RequestTypeUnsubscribe = 0x0400,
RequestTypeNotify = 0x0800,
// Not a request type
RequestTypeResponse = 0x1000

} RequestType;

Expand Down Expand Up @@ -75,7 +82,6 @@ typedef enum

} ResponseType;


typedef struct
{
const char *pszExtension;
Expand Down Expand Up @@ -162,7 +168,7 @@ class UPNP_PUBLIC HTTPRequest

void ProcessRequestLine ( const QString &sLine );
bool ProcessSOAPPayload ( const QString &sSOAPAction );
void ExtractMethodFromURL( );
void ExtractMethodFromURL( ); // Service method, not HTTP method

QString GetResponseStatus ( void );
QString GetResponseType ( void );
Expand Down
62 changes: 58 additions & 4 deletions mythtv/libs/libmythupnp/httpserver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,55 @@

using namespace std;


/**
* \brief Handle an OPTIONS request
*/
bool HttpServerExtension::ProcessOptions(HTTPRequest* pRequest)
{
pRequest->m_eResponseType = ResponseTypeHeader;
pRequest->m_nResponseStatus = 405; // Method Not Available

QStringList allowedMethods;
if (m_nSupportedMethods & RequestTypeGet)
allowedMethods.append("GET");
if (m_nSupportedMethods & RequestTypeHead)
allowedMethods.append("HEAD");
if (m_nSupportedMethods & RequestTypePost)
allowedMethods.append("POST");
// if (m_nSupportedMethods & RequestTypePut)
// allowedMethods.append("PUT");
// if (m_nSupportedMethods & RequestTypeDelete)
// allowedMethods.append("DELETE");
// if (m_nSupportedMethods & RequestTypeConnect)
// allowedMethods.append("CONNECT");
if (m_nSupportedMethods & RequestTypeOptions)
allowedMethods.append("OPTIONS");
// if (m_nSupportedMethods & RequestTypeTrace)
// allowedMethods.append("TRACE");
if (m_nSupportedMethods & RequestTypeMSearch)
allowedMethods.append("M-SEARCH");
if (m_nSupportedMethods & RequestTypeSubscribe)
allowedMethods.append("SUBSCRIBE");
if (m_nSupportedMethods & RequestTypeUnsubscribe)
allowedMethods.append("UNSUBSCRIBE");
if (m_nSupportedMethods & RequestTypeNotify)
allowedMethods.append("NOTIFY");

if (!allowedMethods.isEmpty())
{
pRequest->SetResponseHeader("Allow", allowedMethods.join(", "));
return true;
}

LOG(VB_GENERAL, LOG_ERR, QString("HttpServerExtension::ProcessOptions(): "
"Error: No methods supported for "
"extension - %1").arg(m_sName));

return false;
}


/////////////////////////////////////////////////////////////////////////////
/////////////////////////////////////////////////////////////////////////////
//
Expand Down Expand Up @@ -308,7 +357,10 @@ void HttpServer::DelegateRequest(HTTPRequest *pRequest)
{
try
{
bProcessed = list[ nIdx ]->ProcessRequest(pRequest);
if (pRequest->m_eType == RequestTypeOptions)
bProcessed = list[ nIdx ]->ProcessOptions(pRequest);
else
bProcessed = list[ nIdx ]->ProcessRequest(pRequest);
}
catch(...)
{
Expand All @@ -324,7 +376,10 @@ void HttpServer::DelegateRequest(HTTPRequest *pRequest)
{
try
{
bProcessed = (*it)->ProcessRequest(pRequest);
if (pRequest->m_eType == RequestTypeOptions)
bProcessed = (*it)->ProcessOptions(pRequest);
else
bProcessed = (*it)->ProcessRequest(pRequest);
}
catch(...)
{
Expand Down Expand Up @@ -443,8 +498,7 @@ void HttpWorker::run(void)

try
{
while (m_httpServer.IsRunning() && bKeepAlive && pSocket &&
pSocket->isValid() &&
while (m_httpServer.IsRunning() && bKeepAlive && pSocket->isValid() &&
pSocket->state() == QAbstractSocket::ConnectedState)
{
// We set a timeout on keep-alive connections to avoid blocking
Expand Down
18 changes: 16 additions & 2 deletions mythtv/libs/libmythupnp/httpserver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ class QSslKey;
class QSslCertificate;
class QSslConfiguration;

typedef enum
{
cpLocalNoAuth = 0x00, // Can only be accessed locally, but no authentication is required
cpLocalAuth = 0x01, // Can only be accessed locally, authentication is required
cpRemoteAuth = 0x02 // Can be accessed remotely, authentication is required
} ContentProtection;

/////////////////////////////////////////////////////////////////////////////
/////////////////////////////////////////////////////////////////////////////
//
Expand All @@ -68,16 +75,23 @@ class UPNP_PUBLIC HttpServerExtension : public QObject
QString m_sName;
QString m_sSharePath;
int m_nSocketTimeout; // Extension may wish to adjust the default e.g. UPnP

uint m_nSupportedMethods; // Bit flags - HTTP::RequestType

public:

HttpServerExtension( const QString &sName, const QString &sSharePath )
: m_sName( sName ), m_sSharePath( sSharePath ), m_nSocketTimeout(-1) {};
HttpServerExtension( const QString &sName, const QString &sSharePath)
: m_sName( sName ), m_sSharePath( sSharePath ),
m_nSocketTimeout(-1),
m_nSupportedMethods((RequestTypeGet | RequestTypePost | // Defaults, extensions may extend the list
RequestTypeHead | RequestTypeOptions)) {};

virtual ~HttpServerExtension() {};

virtual bool ProcessRequest(HTTPRequest *pRequest) = 0;

virtual bool ProcessOptions(HTTPRequest *pRequest);

virtual QStringList GetBasePaths() = 0;

virtual int GetSocketTimeout() const { return m_nSocketTimeout; }// -1 = Use config value
Expand Down
1 change: 1 addition & 0 deletions mythtv/libs/libmythupnp/ssdp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,7 @@ SSDPExtension::SSDPExtension( int nServicePort , const QString &sSharePath)
: HttpServerExtension( "SSDP" , sSharePath),
m_nServicePort(nServicePort)
{
m_nSupportedMethods |= (RequestTypeMSearch | RequestTypeNotify);
m_sUPnpDescPath = UPnp::GetConfiguration()->GetValue( "UPnP/DescXmlPath",
m_sSharePath );
}
Expand Down
4 changes: 3 additions & 1 deletion mythtv/libs/libmythupnp/upnpsubscription.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ UPNPSubscription::UPNPSubscription(const QString &share_path, int port)
: HttpServerExtension("UPnPSubscriptionManager", share_path),
m_subscriptionLock(QMutex::Recursive), m_callback(QString("NOTSET"))
{
m_nSupportedMethods = (uint)RequestTypeNotify; // Only NOTIFY supported

QHostAddress addr;
if (!UPnp::g_IPAddrList.isEmpty())
addr = UPnp::g_IPAddrList.at(0);
Expand Down Expand Up @@ -188,7 +190,7 @@ bool UPNPSubscription::ProcessRequest(HTTPRequest *pRequest)

QString nt = pRequest->m_mapHeaders["nt"];
QString nts = pRequest->m_mapHeaders["nts"];
bool no = pRequest->m_sRawRequest.startsWith("NOTIFY");
bool no = (pRequest->m_eType == RequestTypeNotify);

if (nt.isEmpty() || nts.isEmpty() || !no)
{
Expand Down

0 comments on commit 373e52c

Please sign in to comment.