Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(341)

Unified Diff: content/browser/renderer_host/media/midi_host.cc

Issue 68353002: Use MIDIMessageQueue/IsValidWebMIDIData for MIDI byte stream validation (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address comment Created 7 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/renderer_host/media/midi_host.cc
diff --git a/content/browser/renderer_host/media/midi_host.cc b/content/browser/renderer_host/media/midi_host.cc
index 934d5a21f1839fa97a5740fc60d28bba1936a05a..976ba5bc34991283d9bc557f728c01a1e47086f4 100644
--- a/content/browser/renderer_host/media/midi_host.cc
+++ b/content/browser/renderer_host/media/midi_host.cc
@@ -16,6 +16,7 @@
#include "content/public/browser/media_observer.h"
#include "content/public/browser/user_metrics.h"
#include "media/midi/midi_manager.h"
+#include "media/midi/midi_message_util.h"
using media::MIDIManager;
using media::MIDIPortInfoList;
@@ -36,6 +37,7 @@ namespace content {
MIDIHost::MIDIHost(int renderer_process_id, media::MIDIManager* midi_manager)
: renderer_process_id_(renderer_process_id),
+ has_sys_ex_permission_(false),
midi_manager_(midi_manager),
sent_bytes_in_flight_(0),
bytes_sent_since_last_acknowledgement_(0) {
@@ -75,6 +77,12 @@ void MIDIHost::OnStartSession(int client_id) {
if (success) {
input_ports = midi_manager_->input_ports();
output_ports = midi_manager_->output_ports();
+ received_messages_queues_.clear();
+ received_messages_queues_.resize(input_ports.size());
+ // ChildSecurityPolicy is set just before OnStartSession by
+ // MIDIDispatcherHost. So we can safely cache the policy.
+ has_sys_ex_permission_ = ChildProcessSecurityPolicyImpl::GetInstance()->
+ CanSendMIDISysExMessage(renderer_process_id_);
}
}
@@ -94,32 +102,26 @@ void MIDIHost::OnSendData(uint32 port,
if (data.empty())
return;
- base::AutoLock auto_lock(in_flight_lock_);
-
- // Sanity check that we won't send too much.
- if (sent_bytes_in_flight_ > kMaxInFlightBytes ||
- data.size() > kMaxInFlightBytes ||
- data.size() + sent_bytes_in_flight_ > kMaxInFlightBytes)
+ // Blink running in a renderer checks permission to raise a SecurityError
+ // in JavaScript. The actual permission check for security purposes
+ // happens here in the browser process.
+ if (!has_sys_ex_permission_ &&
+ (std::find(data.begin(), data.end(), kSysExMessage) != data.end())) {
+ RecordAction(UserMetricsAction("BadMessageTerminate_MIDI"));
+ BadMessageReceived();
return;
-
- if (data[0] >= kSysExMessage) {
- // Blink running in a renderer checks permission to raise a SecurityError in
- // JavaScript. The actual permission check for security perposes happens
- // here in the browser process.
- if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanSendMIDISysExMessage(
- renderer_process_id_)) {
- RecordAction(UserMetricsAction("BadMessageTerminate_MIDI"));
- BadMessageReceived();
- return;
- }
}
- midi_manager_->DispatchSendMIDIData(
- this,
- port,
- data,
- timestamp);
+ if (!media::IsValidWebMIDIMessage(data))
+ return;
+ base::AutoLock auto_lock(in_flight_lock_);
+ // Sanity check that we won't send too much data.
+ // TODO(yukawa): Consider to send an error event back to the renderer
+ // after some future discussion in W3C.
+ if (data.size() + sent_bytes_in_flight_ > kMaxInFlightBytes)
+ return;
+ midi_manager_->DispatchSendMIDIData(this, port, data, timestamp);
sent_bytes_in_flight_ += data.size();
}
@@ -130,20 +132,29 @@ void MIDIHost::ReceiveMIDIData(
double timestamp) {
TRACE_EVENT0("midi", "MIDIHost::ReceiveMIDIData");
- // Check a process security policy to receive a system exclusive message.
- if (length > 0 && data[0] >= kSysExMessage) {
- if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanSendMIDISysExMessage(
- renderer_process_id_)) {
- // MIDI devices may send a system exclusive messages even if the renderer
- // doesn't have a permission to receive it. Don't kill the renderer as
- // OnSendData() does.
- return;
- }
- }
+ if (received_messages_queues_.size() <= port)
+ return;
- // Send to the renderer.
- std::vector<uint8> v(data, data + length);
- Send(new MIDIMsg_DataReceived(port, v, timestamp));
+ // Lazy initialization
+ if (received_messages_queues_[port] == NULL)
+ received_messages_queues_[port] = new media::MIDIMessageQueue(true);
+
+ received_messages_queues_[port]->Add(data, length);
+ std::vector<uint8> message;
+ while (true) {
+ received_messages_queues_[port]->Get(&message);
+ if (message.empty())
+ break;
+
+ // MIDI devices may send a system exclusive messages even if the renderer
+ // doesn't have a permission to receive it. Don't kill the renderer as
+ // OnSendData() does.
+ if (message[0] == kSysExMessage && !has_sys_ex_permission_)
+ continue;
+
+ // Send to the renderer.
+ Send(new MIDIMsg_DataReceived(port, message, timestamp));
+ }
}
void MIDIHost::AccumulateMIDIBytesSent(size_t n) {

Powered by Google App Engine
This is Rietveld 408576698