> > > I can still make this an array and pass a std::vector<std::string> into 
> > > al_trigger_class function?
> > 
> > Maybe 256 chars are enough at the moment. If other people complain in the future, we can 
> > re-visit.
> > 
> > Stefan
> 
> Thinking about it, an array of maybe 80 character would give enough space for a name, a tag 
> and phone number. Do I need to budget memory very strictly? Would 32 entries of 80 
> characters be too much? 

On that level memory is cheap.

Stefan

I've updated the branch / pull request to use an array of 10 entries (80 chars each). 32 felt a 
little overkill when I saw it on screen, but absolutely happy to set it to any number you 
recommend.

The array gets flattened out when an alarm is triggered, currently the formatting produces

AlarmClass : AlarmMessage (Flattened List Of Users Responsible Array With Space Separators)

If experiments want to use Discord / Slack / Mattermost tags and or add phone numbers, that 
should fit in 80 characters

This list of responsible being attached to alarm message strings will be great for the 
mmessenger, however, perhaps its going to generate very long messages for the speaker programs 
(web interface and mlxspeaker ):

AlarmClass : AlarmMessage (ResponsibleUser1 ResponsibleUser2 ResponsibleUser3 ResponsibleUser4 
... ResponsibleUser4)

especially if people put in user tags or emergency contact details...

Should we add a key word or character for the programs that create audio to parse that silence 
the list of responsible users? I'd be tempted to use a single character but there is a risk 
users might have that in a custom alarm message. Maybe something usual like the 'bel' 
character? '|'? 

Perhaps use the string 'Responsible:' or 'Users:' to trim out the Users Responsible list from 
the message string?

AlarmClass : AlarmMessage Responsible:(ResponsibleUser1 ResponsibleUser2 ResponsibleUser3 
ResponsibleUser4 ... ResponsibleUser4)

AlarmClass : AlarmMessage Users:(ResponsibleUser1 ResponsibleUser2 ResponsibleUser3 
ResponsibleUser4 ... ResponsibleUser4)

> This list of responsible being attached to alarm message strings ...

This is a great idea. But I think we do not need to artificially limit ourselves
to string and array lengths.

The code in alarm.c should be changes to use std::string and std::vector<std::string> (STRING_LIST 
#define), db_get_record() should be replaced with individual ODB reads (that's what it does behind 
the scenes, but in a non-type and -size safe way).

I think the web page code will work correctly, it does not care about string lengths.

K.O.

> > This list of responsible being attached to alarm message strings ...
> 
> This is a great idea. But I think we do not need to artificially limit ourselves
> to string and array lengths.
> 
> The code in alarm.c should be changes to use std::string and std::vector<std::string> (STRING_LIST 
> #define), db_get_record() should be replaced with individual ODB reads (that's what it does behind 
> the scenes, but in a non-type and -size safe way).
> 
> I think the web page code will work correctly, it does not care about string lengths.
> 
> K.O.

Ok, I'm working on this... I see a design choice to make, 1. Keep 'ALARM' as a struct, or 2. Replace ALARM struct with a class (keeping memory layout the same). Since we are adding STRING_LIST, I'd lean towards a C++ style with a class

1. Keep 'ALARM' as a struct:
Get rid of ALARM_ODB_STR for the default values
Add static functions (that take a hkey) to interact with the ODB (to save duplicating logic in al_reset_alarm, al_check, al_define_odb_alarm and al_trigger_alarm)

2. Replace ALARM struct with a class:
default ctor: Do nothing special (so it behaves like the old struct)
default dtor: std::vector<std::string> dtor will

It seems an opportunity to convert the alarm struct to a class with member functions that take hkey pointers?

default ctor: Do nothing special (so it behaves like the old struct)
default dtor: std::vector<std::string> dtor will
SetToDefault: This is where default values are hard coded (functionally replace ALARM_ODB_STR )

> > This list of responsible being attached to alarm message strings ...
> 
> This is a great idea. But I think we do not need to artificially limit ourselves
> to string and array lengths.
> 
> The code in alarm.c should be changes to use std::string and std::vector<std::string> (STRING_LIST 
> #define), db_get_record() should be replaced with individual ODB reads (that's what it does behind 
> the scenes, but in a non-type and -size safe way).
> 
> I think the web page code will work correctly, it does not care about string lengths.
> 
> K.O.

Auto growing lists is an excellent plan. I am making decent progress and should have something to 
report soon

> > > This list of responsible being attached to alarm message strings ...
> > 
> > This is a great idea. But I think we do not need to artificially limit ourselves
> > to string and array lengths.
> > 
> > The code in alarm.c should be changes to use std::string and std::vector<std::string> (STRING_LIST 
> > #define), db_get_record() should be replaced with individual ODB reads (that's what it does behind 
> > the scenes, but in a non-type and -size safe way).
> > 
> > I think the web page code will work correctly, it does not care about string lengths.
> > 
> > K.O.
> 
> Auto growing lists is an excellent plan. I am making decent progress and should have something to 
> report soon

This has sent me down a little rabbit hole, and I'd like to check in with efforts to improve the efficiency and simplicity of the alarm code.

I can keep with the current 'C' style of the alarm.cxx code, replace struct read and writes to the odb with individual odb entries... put functions in alarm.cxx to create, read and write to the odb... 

If we go this 'C' style route, then I'll have duplication of the 'users responsible' setters and getter functions for structs ALARM and PROGRAM_INFO

What would the MIDAS developers thing of creating classes for ALARM and PROGRAM_INFO (I am thinking for binary compatibilities of not touching ALARM and PROGRAM_INFO structs, and inheriting from them:

class UsersResponsible
{
   public:
      STRING_LIST fUsersResponsible;
}

class Alarm: public ALARM, public UsersResponsible
{
}

class ProgramInfo: public PROGRAM_INFO, public UsersResponsible
{
}

Each of these three classes would have member to functions to Create, Read and Write to the ODB. We could get rid of the PROGRAM_INFO_STR precompiler macro and instead have a SetToDefault member function. 

It seems clear we should set the ODB path in the constructor of Alarm and ProgramInfo

When I wrote the musbstd.h "open" method, I kind of punted on the problem of
handling multiple identical USB devices. Instead of a real solution, I added an
"instance" parameter, which allows one to "open" the "first", "second", etc USB
device, as listed in a magic random system dependant order.

Normally, USB devices are identified by two 16-bit integers: manufacturer ID and
product ID (i.e. as reported by "lsusb"). This works well until one has more
than one "identical" device. Two years ago, I had 5 identical USB cameras
(optical alignement system for TRIUMF-TWIST); last year, I had multiple USB
serial adapters; today I have two identical USB-TPC interfaces.

Most of the time, the devices are plugged into the same USB ports, so
theoretically, one should be able to tell exactly which one is which ("upstream
camera is plugged into port 1, downstream camera is plugged into port 2"). But
in the magic system dependant enumeration order, they keep moving around,
depending on the order of enumeration, history of powering up and down, phase of
the Moon, etc.

So my generic "musbstd" method of "open first", "open second", etc turned out to
be completely disfunctional.

So far, I am unable to come up with a system independant solution. But I have a
solution for Linux and maybe for MacOSX:

1) on Linux, I can use the information parsed from /proc/bus/usb/devices to say
"please open the USB device on USB bus 1, port 1", the so called USB device
"path", as seen in the system log and in /sys/bus/usb/devices.

2) on MacOSX, I was unable to find a way to discover the USB topology, but they
seem to maintain an uint32_t "location", which they promise to keep at least
across reboots (did not check this yet).

3) Windows I did not look at yet.

So we have a choice:

a) use system dependant "musb_open_linux(usbpath,vendor,product)",
"musb_open_macosx(???,vendor,product)", etc

b) create order out of chaos by manually keeping a map of "instances" (first,
second, third device) to "persistant addresses". On Linux, it would be a file
containing something like this: "USB-TPC-0 is on bus1-port1, USB-TPC-1 is on
bus1-port2". Then again, I can say "please open USB-TPC interface instance 0" or
"instance 1", etc. There is a small difficulty with dealing with devices
temporarily or permanantly going away, or changing physical addresses ("I moved
the USB device from port 1 to port 3"). This could be handled by telling the
user "hmm... USB topology has changed, please delete the map file and try
again", or we could come up with something more user friendly.

Any thoughts?

P.S. For my immediate need (I need this tomorrow), I will write a
musb_open_linux(usbpath,vendor,product) function.

K.O.

> Any thoughts?

I got an idea of how to solve this problem in an OS-independent manner. The USB
devices and hubs form a tree, like this

  Root  HUB
  0   1   2
  |   |   \__...
  |   \___
 DevY     \
         HUB
        0 1 2
        |   |
        |  DevX
       HUB
      0 1 2
          |
        DevZ

This tree can be considered as an ordered tree, if you read it from left to right.
In that order, the devices are orderd

DevY - DevZ - DevX

Since the devices are ordered, the "instance" parameter from musb_init can be used
to identify them uniquely, like

instance==0   => DevY
instance==1   => DevZ
instance==2   => DevX

So I would say that we can use the current API using the "instance" parameter to
uniquely access a device. All we have to do is to build that tree, sort it, and then
use the instance parameter as an entry to that tree. The sorting takes care of
different ordering, which can happen during enumeration (depeding on power-up
sequence, phase of the Moon etc.). So if you have three devices like above, DevZ
should alway be at "instance==1". The only problem is if you unplug DevY for
example, then you get the map

instance==0   => DevZ
instance==1   => DevX

which is different from above. But if you have a different number of devices, you
likely have to change your frontend cody anyhow, so you can change the device
mapping there as well. 

In order to simplify the code, I would not build a complete tree and sort it, but
scan the whole tree hierarchically, i.e. look at

Bus1/Port1
Bus1/Port2
Bus1/...
Bus2/Port1
Bus2/Port2
...

Since there is a maximum of toal 127 USB devices, this scan should be pretty quick.
If you find a device with matching vendor and product ID, you increment an internal
counter. If that counter matches your instance parameter, you open that device.

The ultimate solution of course is to put an additional address into each device, so
you can distinguish them easily. For a out-of-the box Web cam you probably have no
chance, but for the home-made MSCB nodes I put such an address into each node, so I
can distinguish them even if the have the same product and vendor ID.

> root -l
root> TFile *f = new TFile("run00064.root")
root> TTree *t = f->Get("Trigger")
root> t->StartViewer() // look at the ROOT TTree
root> t->MakeSelector() // generates Trigger.h, Trigger.C

edit run.C, the main program:
{
gROOT->Reset();
TFile f("data/run00064.root");
TTree *t = f.Get("Trigger");

TH1D* adc8 = new TH1D("adc8","ADC8",1500,0,1500-1);
TH1D* tdc2 = new TH1D("tdc2","TDC2",1500,0,1500-1);
TH2D* h12 = new TH2D("h2","ADC8 vs TDC2",100,0,1500,100,0,1500);
TH2D* h12cut = new TH2D("h2cut","ADC8 vs TDC2",50,0,1000-1,50,0,1500);

TSelector *s = TSelector::GetSelector("Trigger.C");
t->Process(s);

adc8->Draw();
tdc2->Draw();
h12->Draw();
h12cut->Draw();
}

edit Trigger.C:

Bool_t Trigger::ProcessCut(Int_t entry)
{
  fChain->GetTree()->GetEntry(entry);
  if (entry%100 == 0) printf("entry %d\r",entry);
  return kTRUE;
}

void Trigger::ProcessFill(Int_t entry)
{
  adc8->Fill(ADCS_ADCS[8]);
  tdc2->Fill(TDCS_TDCS[2]);
  h12->Fill(TDCS_TDCS[2],ADCS_ADCS[8]);
  if (ADCS_ADCS[8] > 100)
    h12cut->Fill(TDCS_TDCS[2],ADCS_ADCS[8]);
}

Run the analysis:

root -l
root> .x run.C

K.O.

> root -l
root> TFile *f = new TFile("run00064.root")
root> TTree *t = f->Get("Trigger")
root> t->StartViewer() // look at the ROOT TTree
root> t->MakeSelector() // generates Trigger.h, Trigger.C

edit run.C, the main program:
{
gROOT->Reset();
TFile f("data/run00064.root");
TTree *t = f.Get("Trigger");

TH1D* adc8 = new TH1D("adc8","ADC8",1500,0,1500-1);
TH1D* tdc2 = new TH1D("tdc2","TDC2",1500,0,1500-1);
TH2D* h12 = new TH2D("h2","ADC8 vs TDC2",100,0,1500,100,0,1500);
TH2D* h12cut = new TH2D("h2cut","ADC8 vs TDC2",50,0,1000-1,50,0,1500);

TSelector *s = TSelector::GetSelector("Trigger.C");
t->Process(s);

adc8->Draw();
tdc2->Draw();
h12->Draw();
h12cut->Draw();
}

edit Trigger.C:

Bool_t Trigger::ProcessCut(Int_t entry)
{
  fChain->GetTree()->GetEntry(entry);
  if (entry%100 == 0) printf("entry %d\r",entry);
  return kTRUE;
}

void Trigger::ProcessFill(Int_t entry)
{
  adc8->Fill(ADCS_ADCS[8]);
  tdc2->Fill(TDCS_TDCS[2]);
  h12->Fill(TDCS_TDCS[2],ADCS_ADCS[8]);
  if (ADCS_ADCS[8] > 100)
    h12cut->Fill(TDCS_TDCS[2],ADCS_ADCS[8]);
}

Run the analysis:

root -l
root> .x run.C

K.O.

> root -l
root> TFile *f = new TFile("run00064.root")
root> TTree *t = f->Get("Trigger")
root> t->StartViewer() // look at the ROOT TTree
root> t->MakeSelector() // generates Trigger.h, Trigger.C

edit run.C, the main program:
{
gROOT->Reset();
TFile f("data/run00064.root");
TTree *t = f.Get("Trigger");

TH1D* adc8 = new TH1D("adc8","ADC8",1500,0,1500-1);
TH1D* tdc2 = new TH1D("tdc2","TDC2",1500,0,1500-1);
TH2D* h12 = new TH2D("h2","ADC8 vs TDC2",100,0,1500,100,0,1500);
TH2D* h12cut = new TH2D("h2cut","ADC8 vs TDC2",50,0,1000-1,50,0,1500);

TSelector *s = TSelector::GetSelector("Trigger.C");
t->Process(s);

adc8->Draw();
tdc2->Draw();
h12->Draw();
h12cut->Draw();
}

edit Trigger.C:

Bool_t Trigger::ProcessCut(Int_t entry)
{
  fChain->GetTree()->GetEntry(entry);
  if (entry%100 == 0) printf("entry %d\r",entry);
  return kTRUE;
}

void Trigger::ProcessFill(Int_t entry)
{
  adc8->Fill(ADCS_ADCS[8]);
  tdc2->Fill(TDCS_TDCS[2]);
  h12->Fill(TDCS_TDCS[2],ADCS_ADCS[8]);
  if (ADCS_ADCS[8] > 100)
    h12cut->Fill(TDCS_TDCS[2],ADCS_ADCS[8]);
}

Run the analysis:

root -l
root> .x run.C

K.O.

Updating the instructions to ROOT version 3.10.2. Example is from TRIUMF-KOPIO
tree analysis.

shell> root -l
root> TFile *f = new TFile("run00064.root")
root> Trigger->MakeSelector("TriggerSelector")  // "Trigger" is the tree name
inside the root file. Generates TriggerSelector.h and TriggerSelector.cpp

= edit run.C, the main program:

{
gROOT->Reset();
TSelector *s = TSelector::GetSelector("TriggerSelector.C");

TChain chain("Trigger"); // "Trigger" is the tree name inside the root files
chain.Add("run03016.root"); // can chain multiple files

TH1D* tdc2 = new TH1D("tdc2","TDC2",1500,0,1500-1);

chain.Process(s,"",500); // process 500 events
//chain.Process(s); // or process all events

tdc2->Draw();
}

= edit TriggerSelector.h:

in the TriggerSelector class members, i.e. "UInt_t TDC1_TDC1[47];" edit the
array size to be bigger than the maximum possible bank size

= edit TriggerSelector.C:

Bool_t TriggerSelector::Process(Int_t entry)
{
  fChain->GetTree()->GetEntry(entry);

  if (entry%100 == 0)
    printf("process %d, nTDC %3d, 0x%08x\n",entry,TDC1_nTDC1,TDC1_TDC1[1]);

  tdc2->Fill(TDC1_nTDC1);
  return kTRUE;
}

= Run the analysis:

shell> root -l
root> .x run.C
 
K.O.

Updating the instructions to ROOT version 3.10.2. Example is from TRIUMF-KOPIO
tree analysis.

shell> root -l
root> TFile *f = new TFile("run00064.root")
root> Trigger->MakeSelector("TriggerSelector")  // "Trigger" is the tree name
inside the root file. Generates TriggerSelector.h and TriggerSelector.cpp

= edit run.C, the main program:

{
gROOT->Reset();
TSelector *s = TSelector::GetSelector("TriggerSelector.C");

TChain chain("Trigger"); // "Trigger" is the tree name inside the root files
chain.Add("run03016.root"); // can chain multiple files

TH1D* tdc2 = new TH1D("tdc2","TDC2",1500,0,1500-1);

chain.Process(s,"",500); // process 500 events
//chain.Process(s); // or process all events

tdc2->Draw();
}

= edit TriggerSelector.h:

in the TriggerSelector class members, i.e. "UInt_t TDC1_TDC1[47];" edit the
array size to be bigger than the maximum possible bank size

= edit TriggerSelector.C:

Bool_t TriggerSelector::Process(Int_t entry)
{
  fChain->GetTree()->GetEntry(entry);

  if (entry%100 == 0)
    printf("process %d, nTDC %3d, 0x%08x\n",entry,TDC1_nTDC1,TDC1_TDC1[1]);

  tdc2->Fill(TDC1_nTDC1);
  return kTRUE;
}

= Run the analysis:

shell> root -l
root> .x run.C
 
K.O.

Updating the instructions to ROOT version 3.10.2. Example is from TRIUMF-KOPIO
tree analysis.

shell> root -l
root> TFile *f = new TFile("run00064.root")
root> Trigger->MakeSelector("TriggerSelector")  // "Trigger" is the tree name
inside the root file. Generates TriggerSelector.h and TriggerSelector.cpp

= edit run.C, the main program:

{
gROOT->Reset();
TSelector *s = TSelector::GetSelector("TriggerSelector.C");

TChain chain("Trigger"); // "Trigger" is the tree name inside the root files
chain.Add("run03016.root"); // can chain multiple files

TH1D* tdc2 = new TH1D("tdc2","TDC2",1500,0,1500-1);

chain.Process(s,"",500); // process 500 events
//chain.Process(s); // or process all events

tdc2->Draw();
}

= edit TriggerSelector.h:

in the TriggerSelector class members, i.e. "UInt_t TDC1_TDC1[47];" edit the
array size to be bigger than the maximum possible bank size

= edit TriggerSelector.C:

Bool_t TriggerSelector::Process(Int_t entry)
{
  fChain->GetTree()->GetEntry(entry);

  if (entry%100 == 0)
    printf("process %d, nTDC %3d, 0x%08x\n",entry,TDC1_nTDC1,TDC1_TDC1[1]);

  tdc2->Fill(TDC1_nTDC1);
  return kTRUE;
}

= Run the analysis:

shell> root -l
root> .x run.C
 
K.O.

With recent changes to mana.c, creation of user ROOT objects in the MIDAS
analyser has changed. Here is the new example code for creating ROOT objects
that are visible in ROODY and are saved into the histogram file.

1) in the "global" context (outside of any function)

#include <TH1D.h>
#include <TProfile.h>

static TH1D* gMyHist1 = 0;
static TProfile* gMyHist2 = 0;

2) In the analyzer "init" or "begin run" method, create the histogram:

//extern TFolder *gManaHistosFolder; // from midas.h
gMyHist1 = new TH1D("gMyHist1",...);
gMyHist2 = new TProfile("gMyHist2",...);
gManaHistosFolder->Add(gMyHist1);
gManaHistosFolder->Add(gMyHist2);

(note: this will produce an warning about "possible memory leak")

3) In the per-event method, fill the histograms

gMyHist1->Fill(x);
gMyHist2->Fill(x,y);

4) In the Makefile, where you compile the frontend, add "-DUSE_ROOT" right after
"-I$(ROOTSYS)/include"

K.O.

> (preliminary, untested. I will keep this updated as I get testing feedback)
> 
> With recent changes to mana.c, creation of user ROOT objects in the MIDAS
> analyser has changed. Here is the new example code for creating ROOT objects
> that are visible in ROODY and are saved into the histogram file.
> 
> 1) in the "global" context (outside of any function)
> 
> #include <TH1D.h>
> #include <TProfile.h>
> 
> static TH1D* gMyHist1 = 0;
> static TProfile* gMyHist2 = 0;
> 
> 2) In the analyzer "init" or "begin run" method, create the histogram:
> 
> //extern TFolder *gManaHistosFolder; // from midas.h
> gMyHist1 = new TH1D("gMyHist1",...);
> gMyHist2 = new TProfile("gMyHist2",...);
> gManaHistosFolder->Add(gMyHist1);
> gManaHistosFolder->Add(gMyHist2);
> 
> (note: this will produce an warning about "possible memory leak")
> 
> 3) In the per-event method, fill the histograms
> 
> gMyHist1->Fill(x);
> gMyHist2->Fill(x,y);
> 
> K.O.


the book functions provide a convenient place to check against object duplication
and memory leaks etc., and a place to ensure that consistent subfolders are being
used.  eg. a while back we decided that TCutGs should be in a "cuts" subfolder.

To extend the booking to TProfile is fairly easy.  In fact if you want to
use the simple constructor TProfile::TProfile (const char *, const char *, Int_t,
Axis_t, Axis_t), then you could infact just use h1_book<TProfile>.

It now seems to me that the names h1_book, h2_book, cut_book are all too long
and even more upsetting are inconsistent.  Some of them are templates (most) and
some are not.  Perhaps they should all be templates, and all have the same name.
The attached patch accomplishes this (without deleting the old names).  With this
patch you can now do

gMyHist1 = book<TProfile>( "gMyHist2",...);

New book templates are needed when you (1) wish to change the subfolder, or (2)
need to use a different argument list in the constructor.  If you need help with
this for the TProfile constructors which are different from TH1D constructors then
let me know.  They should be easy to do.

For TGraph at lot depends on how you want to initialise the data points.

Before the days of javascript and ajax and web 2.0, MIDAS introduced "custom pages" for
building graphical display that could show "live" data from MIDAS and that could
have buttons and controls to operate slow controls equipment, etc.

This is how it works:
- entries from ODB /Custom are shown on the MIDAS menu -
an odb entry /Custom/Foo generate a link labeled "Foo"
to a special mhttpd url /CS/Foo.
- access to mhttpd url /CS/Foo invokes show_custom_page()
- show_custom_page() reads the custom page file name from ODB /Custom/Foo
- content of this file is served the web browser (after substituting the <odb> tags with values from ODB).
- in addition, it is possible to store the contents of the custom page in the ODB variable /Custom/Foo itself,
making it easy to edit the custom pages through the web browser (using the mhttpd odb editor).
- (if the value of /Custom/Foo has no "\n", then it's a file name, otherwise it is the page contents).
- if /Custom/Path exists, it is prepended to all file names.
- read more about this here:
https://midas.triumf.ca/MidasWiki/index.php//Custom_ODB_tree
https://midas.triumf.ca/MidasWiki/index.php/Internal_Custom_Page

This method required each custom web page served by mhttpd to have a corresponding
entry in ODB /Custom. Quite tedious for big experiments with a large number
of web pages (in T2K/ND280/FGD, for 1 page per frontend board, these entries
had to be created using a script, no practical to create them manually).

To fix this, in 2015, a modification was made to the code for ODB /Custom/Path. Instead of reading the file name
from ODB /Custom/Foo, the filename was made by adding /Custom/Path and the URL itself:
URL /CS/Foo.html will serve the file ODBValue["/Custom/Path"]/Foo.html. In this scheme,
the ODB entry /Custom/Foo.html is optional and is only needed to create a link in the MIDAS menu.

Commit: https://bitbucket.org/tmidas/midas/commits/5a2ef7d66df353684c4b40882a391b64a068f61f made on 2015-03-19.
Corresponding midas forum entry: https://midas.triumf.ca/elog/Midas/1109 made on 2015-09-09.

This change had the effect of creating 2 different operating modes for custom pages:
- if ODB /Custom/Path was absent (the default case), file names are taken from ODB /Custom/Foo.
- if ODB /Custom/Path is present (it has be created manually), file names are taken from the URLs.

These two modes could not be mixed, some experiments (TRIUMF MUSR/BNMR, CERN ALPHA, etc)
continued to use the old scheme (no /Custom/Path), some experiments switched
to the new scheme (DEAP, etc - TBC?).

(The best I can tell, this also create a security problem were using URLs containing "..", one can
force mhttpd to escape the /Custom/Path filename jail to cause it to serve arbitrary files from
the filesystem. Note that many web browsers remove the ".." entries from URLs, special tricks
are required to exploit this).

Then, in 2017, something called "new custom pages" was added to mhttpd interprete().
commit https://bitbucket.org/tmidas/midas/commits/c574a45c6da1290430f5a85a348fb629596a0de0 made on 2017-07-24
(I cannot find the corresponding explanation on the midas forum).

The best I can tell this code is intended to do the same thing that was done before:
map URLs like http://localhost:8080/Foo.html to file name ODBValue["/Custom/Path"]/Foo.html
and if this file exists, serve it to the browser.

However, the code has a bug - there is no check for absence of ODB /Custom/Path. If it does
not exist (as in experiments that do not use custom pages or use the old-style custom pages without
/Custom/Path), an empty string is used and URLs like http://localhost:8080/Foo.html becomes
mapped to /Foo.html (in the root of the file system, "/"!!!).

This introduced 3 problems:
- http://localhost:8080/etc/hosts serve the system password file (a security problem)
- because this code is before the odb code, it intercepts (and breaks) valid ODB URLs:
- on Linux, the ODB editor URL http://localhost:8080/root instead of showing the ODB root, attempts to serve the local subdirectory "/root" as a file
(fails, /root is a directory on most Linuxes). This is reported here: https://midas.triumf.ca/elog/Midas/1335 (2018-01-10)
- on MacOS, ODB editor URL http://localhost:8080/System instead of showing ODB /System, attempts to serve the local directory "/System" is a file (fails, 
/System is a subdirectory in MacOS). This is reported here: https://bitbucket.org/tmidas/midas/issues/156/mhttpd-odb-editor-cannot-open-system-on 
(2018-12-20)

The problem with broken access to the ODB editor URLs is fixed by commit:
https://bitbucket.org/tmidas/midas/commits/f071bc1daa4dc7ec582586659dd2339f1cd1fa21 (2018-12-21)
https://midas.triumf.ca/elog/Midas/1416

The fix unconditionally creates /Custom/Path and sets it to the value of $MIDAS_DIR or $MIDASSYS.

This breaks experiments that use custom pages with no /Custom/Path (ALPHA, BNMR/BNQR), see
https://bitbucket.org/tmidas/midas/issues/157/odb-custom-is-broken, and this is impossible
to fix by deleting /Custom/Path as it will be created again.

Also the problem with serving the system password file remains, see
https://midas.triumf.ca/elog/Midas/1425

So, a https://en.wikipedia.org/wiki/SNAFU

Since then, we have changed the mhttpd URL scheme: we no longer use URL subdirectories to navigate the ODB editor (we now use &odb_path=xxx)
and to refer to custom files (use now use &cmd=custom&page=xxx).

This liberates the mhttpd URL space for serving custom files without colliding with mhttpd internals and allows us to repair the current situation.

I will propose a solution in a follow-up message.

K.O.

P.S. The actual series of malfunctions is a bit more complicated than I described in this message,
but there is no need for getting to the very bottom bottom bottom of this as we can now do
a fairly clean solution.

Parsing all URL in mhttpd to prevent /etc/passwd etc. to be returned is tricky, because people can use escape sequences etc. Therefore I think it is much better to restrict file access 
on the file system level when opening a file. The only escape there one could have is "..", which can be tested easily. 

Therefore, I propose to restrict file access to two well-defined directories, which is one system directory and one user directory. The system directory should be defined via 
$MIDASSYS/resources, and the user directory should be the experiment directory (as defined in exptab) followed by "resources". So if MIDASSYS equals to /usr/local/midas and the 
experiment directory equals to /home/users/exp for example, we would only have these two directories (and of course the subdirectories within these) served by mhttpd:

$MIDASSYS/resource -> /usr/local/midas/resources
<exptab>/resources -> /home/users/exp/resources

These directories should be hard-wired into mhttpd, and not go through and ODB entry, since otherwise one could manipulate the ODB entries (knowingly or unknowingly) and open a 
back-door. 

If users need a more complex structure, they can put soft links into these directories.

The code which opens a resource file should then first evaluate $MIDASSYS, then add "/resources/", then add the requested file name, make sure that there is no ".." in the file name, 
then open the file. If not existing, do the same for the <exptab>/resources/ directory.

This change will break most experiments, and forces people to move their custom pages to different directories, but I think it's the only clean solution and we just have to bite the 
bullet.

Comments are welcome.

Stefan

Hi Stefan and Konstantin,

I think that this proposal sounds fairly reasonable.  I agree that we might as well move to a secure final solution at this point.  

One comment: since this change would break almost every experiment I have worked on for the last 4 years, it would be nice to add a command-line option to mhttpd that preserves the old /Custom/Path behavior.  This would allow experiments a transition 
period, so that they didn't immediately need to fix their setup.  The command-line option could be clearly marked as obsolete behaviour and could be removed within a year.

Cheers,
Thomas



> Parsing all URL in mhttpd to prevent /etc/passwd etc. to be returned is tricky, because people can use escape sequences etc. Therefore I think it is much better to restrict file access 
> on the file system level when opening a file. The only escape there one could have is "..", which can be tested easily. 
> 
> Therefore, I propose to restrict file access to two well-defined directories, which is one system directory and one user directory. The system directory should be defined via 
> $MIDASSYS/resources, and the user directory should be the experiment directory (as defined in exptab) followed by "resources". So if MIDASSYS equals to /usr/local/midas and the 
> experiment directory equals to /home/users/exp for example, we would only have these two directories (and of course the subdirectories within these) served by mhttpd:
> 
> $MIDASSYS/resource -> /usr/local/midas/resources
> <exptab>/resources -> /home/users/exp/resources
> 
> These directories should be hard-wired into mhttpd, and not go through and ODB entry, since otherwise one could manipulate the ODB entries (knowingly or unknowingly) and open a 
> back-door. 
> 
> If users need a more complex structure, they can put soft links into these directories.
> 
> The code which opens a resource file should then first evaluate $MIDASSYS, then add "/resources/", then add the requested file name, make sure that there is no ".." in the file name, 
> then open the file. If not existing, do the same for the <exptab>/resources/ directory.
> 
> This change will break most experiments, and forces people to move their custom pages to different directories, but I think it's the only clean solution and we just have to bite the 
> bullet.
> 
> Comments are welcome.
> 
> Stefan