Mforns has uploaded a new change for review.
https://gerrit.wikimedia.org/r/196599
Change subject: Add size limit to event url
......................................................................
Add size limit to event url
Today, when the event url is too long, the query string gets
truncated to its first 1014 chars by varnishncsa. This results
in validation errors that can be frequent if the schema is
sufficiently big. The consequence is unnecessary overhead in
the EventLogging back-end and unpredictible variations in the
validation alerting system.
This change adds a check on the client-side to avoid sending
events that are too long. In that case an error is logged in
the console if available. It also adds a check in the dev-server.
Bug: T91918
Change-Id: Ie164a9357cdd599823f64122dcc79fec2198832f
---
M modules/ext.eventLogging.core.js
M server/eventlogging/parse.py
M tests/ext.eventLogging.tests.js
3 files changed, 82 insertions(+), 3 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/EventLogging
refs/changes/99/196599/1
diff --git a/modules/ext.eventLogging.core.js b/modules/ext.eventLogging.core.js
index 6ab72c3..ddf339b 100644
--- a/modules/ext.eventLogging.core.js
+++ b/modules/ext.eventLogging.core.js
@@ -46,6 +46,16 @@
schemas: {},
/**
+ * Maximum length in chars that a beacon url can have.
+ * If a url is longer than that, the log may be truncated
+ * by varnishncsa and result in validation problems.
+ *
+ * @property maxUrlSize
+ * @type Number
+ */
+ maxUrlSize: 1000,
+
+ /**
* Load a schema from the schema registry.
* If the schema does not exist, it will be initialised.
*
@@ -204,6 +214,23 @@
},
/**
+ * Checks whether a beacon url is short enough,
+ * so that it does not get truncated by varnishncsa.
+ *
+ * @param {String} schemaName Canonical schema name.
+ * @param {String} url Beacon url.
+ * @return {String|undefined} The error message in case of
error.
+ *
+ */
+ checkUrlSize: function ( schemaName, url ) {
+ if ( url.length > self.maxUrlSize ) {
+ var message = 'Url exceeds maximum length';
+ mw.track( 'eventlogging.error', mw.format(
'[$1] $2', schemaName, message ) );
+ return message;
+ }
+ },
+
+ /**
* Transfer data to a remote server by making a lightweight HTTP
* request to the specified URL.
*
@@ -228,9 +255,18 @@
* @return {jQuery.Promise} jQuery Promise object for the
logging call.
*/
logEvent: function ( schemaName, eventData ) {
- var event = self.prepare( schemaName, eventData );
- self.sendBeacon( self.makeBeaconUrl( event ) );
- return $.Deferred().resolveWith( event, [ event ]
).promise();
+ var event = self.prepare( schemaName, eventData ),
+ url = self.makeBeaconUrl( event ),
+ sizeError = self.checkUrlSize( schemaName, url
),
+ deferred = $.Deferred();
+
+ if ( !sizeError ) {
+ self.sendBeacon( url );
+ deferred.resolveWith( event, [ event ] );
+ } else {
+ deferred.rejectWith( event, [ event, sizeError
] );
+ }
+ return deferred.promise();
}
};
diff --git a/server/eventlogging/parse.py b/server/eventlogging/parse.py
index c0f8be4..c41136c 100644
--- a/server/eventlogging/parse.py
+++ b/server/eventlogging/parse.py
@@ -65,6 +65,11 @@
# used to anonymize IP addresses.
KEY_LIFESPAN = datetime.timedelta(days=90)
+# Limit to the query string size.
+# If a log has a longer query string, the developer will see an error.
+# This will help the developer avoid log truncation by varnishncsa.
+MAX_QS_SIZE = 900
+
def capsule_uuid(capsule):
"""Generate a UUID for a capsule object.
@@ -100,6 +105,11 @@
"""Decodes a QSON (query-string-encoded JSON) object.
:param qs: Query string.
"""
+ # Raises an error when the query string is longer than MAX_QS_SIZE.
+ if len(qson) > MAX_QS_SIZE:
+ raise ValueError(
+ 'Query string size (%d) is greater than max size (%d)' %
+ (len(qson), MAX_QS_SIZE))
return json.loads(unquote_plus(qson.strip('?;')))
diff --git a/tests/ext.eventLogging.tests.js b/tests/ext.eventLogging.tests.js
index 4678110..b0279d9 100644
--- a/tests/ext.eventLogging.tests.js
+++ b/tests/ext.eventLogging.tests.js
@@ -115,6 +115,39 @@
} );
} );
+
+ $.each( {
+ 'URL size is ok': {
+ size: mw.eventLog.maxUrlSize,
+ expected: undefined
+ },
+ 'URL size is not ok': {
+ size: mw.eventLog.maxUrlSize + 1,
+ expected: 'Url exceeds maximum length'
+ }
+ }, function ( name, params ) {
+ QUnit.test( name, 1, function ( assert ) {
+ var url = new Array( params.size + 1 ).join( 'x' ),
+ result = mw.eventLog.checkUrlSize(
'earthquake', url );
+ assert.deepEqual( result, params.expected, name );
+ } );
+ } );
+
+ QUnit.asyncTest( 'logTooLongEvent', 1, function ( assert ) {
+ var event = {
+ epicenter: 'Valdivia',
+ magnitude: 9.5,
+ article: new Array( mw.eventLog.maxUrlSize + 1).join(
'x' )
+ };
+
+ mw.eventLog.logEvent( 'earthquake', event ).always( function (
e, error ) {
+ QUnit.start();
+ assert.deepEqual( error, 'Url exceeds maximum length',
+ 'logEvent promise resolves with error' );
+ } );
+ } );
+
+
QUnit.test( 'setDefaults', 1, function ( assert ) {
var prepared, defaults;
--
To view, visit https://gerrit.wikimedia.org/r/196599
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie164a9357cdd599823f64122dcc79fec2198832f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/EventLogging
Gerrit-Branch: master
Gerrit-Owner: Mforns <mfo...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
