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

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: Removed unnecessary notation from a header comment, to address scherkus' 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..d6781f61df56e2bb27d04fe0f31dfcd82f3839ea 100644
--- a/content/browser/renderer_host/media/midi_host.cc
+++ b/content/browser/renderer_host/media/midi_host.cc
@@ -16,26 +16,41 @@
#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_queue.h"
+#include "media/midi/midi_message_util.h"
using media::MIDIManager;
using media::MIDIPortInfoList;
+namespace content {
+namespace {
+
// The total number of bytes which we're allowed to send to the OS
// before knowing that they have been successfully sent.
-static const size_t kMaxInFlightBytes = 10 * 1024 * 1024; // 10 MB.
+const size_t kMaxInFlightBytes = 10 * 1024 * 1024; // 10 MB.
// We keep track of the number of bytes successfully sent to
// the hardware. Every once in a while we report back to the renderer
// the number of bytes sent since the last report. This threshold determines
// how many bytes will be sent before reporting back to the renderer.
-static const size_t kAcknowledgementThresholdBytes = 1024 * 1024; // 1 MB.
+const size_t kAcknowledgementThresholdBytes = 1024 * 1024; // 1 MB.
-static const uint8 kSysExMessage = 0xf0;
+const uint8 kSysExMessage = 0xf0;
+const uint8 kEndOfSysExMessage = 0xf7;
-namespace content {
+bool IsDataByte(uint8 data) {
+ return (data & 0x80) == 0;
+}
+
+bool IsSystemRealTimeMessage(uint8 data) {
+ return 0xf8 <= data && data <= 0xff;
+}
+
+} // namespace
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 +90,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 +115,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 (!IsValidWebMIDIData(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 +145,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) {
@@ -165,4 +189,37 @@ void MIDIHost::AccumulateMIDIBytesSent(size_t n) {
}
}
+// static
+bool MIDIHost::IsValidWebMIDIData(const std::vector<uint8>& data) {
+ bool in_sysex = false;
+ size_t waiting_data_length = 0;
+ for (size_t i = 0; i < data.size(); ++i) {
+ const uint8 current = data[i];
+ if (IsSystemRealTimeMessage(current))
+ continue; // Real time message can be placed at any point.
+ if (waiting_data_length > 0) {
+ if (!IsDataByte(current))
+ return false; // Error: |current| should have been data byte.
+ --waiting_data_length;
+ continue; // Found data byte as expected.
+ }
+ if (in_sysex) {
+ if (data[i] == kEndOfSysExMessage)
+ in_sysex = false;
+ else if (!IsDataByte(current))
+ return false; // Error: |current| should have been data byte.
+ continue; // Found data byte as expected.
+ }
+ if (current == kSysExMessage) {
+ in_sysex = true;
+ continue; // Found SysEX
+ }
+ waiting_data_length = media::GetMIDIMessageLength(current);
+ if (waiting_data_length == 0)
+ return false; // Error: |current| should have been a valid status byte.
+ --waiting_data_length; // Found status byte
+ }
+ return waiting_data_length == 0 && !in_sysex;
+}
+
} // namespace content
« no previous file with comments | « content/browser/renderer_host/media/midi_host.h ('k') | content/browser/renderer_host/media/midi_host_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698